Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protobuf access logs #1069

Merged
merged 16 commits into from
Mar 11, 2024
Merged

Protobuf access logs #1069

merged 16 commits into from
Mar 11, 2024

Conversation

Keksoj
Copy link
Member

@Keksoj Keksoj commented Feb 2, 2024

Adding the option to produce access logs in a binary format will be highly beneficial for performance and parseability.

This PR creates a binary type BinaryAccessLog using protobuf, that are emitted with a delimiter for easier parsing.

@Keksoj Keksoj force-pushed the binary-access-logs branch 4 times, most recently from 103abc3 to 42c9313 Compare February 5, 2024 17:11
@Keksoj
Copy link
Member Author

Keksoj commented Feb 5, 2024

@FlorentinDUBOIS should we rather have "binary" or "protobuf" as a configurable option, and for types ? We may introduce other binary types in the future, so we can disambiguate right now.

@Keksoj Keksoj force-pushed the binary-access-logs branch 2 times, most recently from a66f026 to ca64cae Compare February 6, 2024 09:22
@Wonshtrum Wonshtrum force-pushed the binary-access-logs branch 6 times, most recently from 1dbc3c9 to a75010c Compare February 8, 2024 17:02
@Keksoj Keksoj changed the title Binary access logs Protobuf access logs Feb 9, 2024
@Keksoj Keksoj force-pushed the binary-access-logs branch 2 times, most recently from 7122d3c to 8378aa2 Compare February 9, 2024 16:53
@Wonshtrum
Copy link
Member

Wonshtrum commented Feb 17, 2024

I have some todos left:

  • document, change, or delete the use of unsafe static mut (for log call site caches)
  • make sure the new ASCII access log format is "good" and consistent with our internal parsers
  • make sure the ownership duplication is safe and necessary

@Wonshtrum Wonshtrum force-pushed the binary-access-logs branch 2 times, most recently from 35c7aca to 0ca4d2a Compare February 21, 2024 15:37
@Keksoj Keksoj force-pushed the binary-access-logs branch 2 times, most recently from a9d4c3b to 653220b Compare March 1, 2024 16:23
@Keksoj
Copy link
Member Author

Keksoj commented Mar 1, 2024

This is ready for review and mergeable in our opinion.

bin/src/command/requests.rs Outdated Show resolved Hide resolved
bin/src/worker.rs Show resolved Hide resolved
command/examples/bench_logger.rs Outdated Show resolved Hide resolved
command/src/command.proto Show resolved Hide resolved
lib/src/protocol/kawa_h1/parser.rs Show resolved Hide resolved
lib/src/protocol/pipe.rs Outdated Show resolved Hide resolved
lib/src/protocol/pipe.rs Show resolved Hide resolved
Keksoj and others added 16 commits March 11, 2024 17:40
configuration:
- create configurable log_access_format: ascii or binary

new types:
- create protobuf type BinaryAccessLog, with subtypes
- replace lib::logs::Endpoint with proto::command::BinaryEndpoint
- use fixed32 for ipv4 addresses

refactor:
- remove log module in sozu_lib
- isolate SessionMetrics::register_end_of_session

transfer types to sozu_command_lib::logging:
- CachedTags
- RequestRecord
- LogDuration
- LogContext

avoid allocation:
- create unsafe trait DuplicateOwnership to display access logs
- intermediate buffer in the logger

Rewrite log macros:
- remove separate TAG RefCell
- move log backends and directives to InnerLogger
- allow borrow of Logger::tag and Logger::inner simultaneously
- check for Logger::enabled as soon as possible
- deduplicate log macros with/without arguments
- use scoped macro names instead of the alphabetic list

Co-Authored-By: Eloi DEMOLIS <[email protected]>
- Refactor the Logger with the new more powerful macro prompt_log
- Explicitely take ownership of RequestRecord fields in access log
  macros, the user can decide the best  way to pass them
- Add logger specific fields in the RequestRecord
- Add basic colors to logs and access logs, configurable in the condig,
  checked at runtime
- Cache the Logger::enabled result at call site

Signed-off-by: Eloi DEMOLIS <[email protected]>
- Handle Logging command at worker level (not proxy)
- parse_logging_spec returns a vec of parsing errors
- Logging command takes a string for logging_filter instead of LogLevel
- Logging command fails uppon any errors of parse_logging_spec
- Logger::init ignores parse_logging_spec errors
- Reorganise some code handling commands
- Generate access log on abrupt client deconnection

Signed-off-by: Eloi DEMOLIS <[email protected]>
the point is to use a structure instead of a text,
for an easier parsing of the access log of the access logss
@FlorentinDUBOIS FlorentinDUBOIS merged commit 5e2c6d7 into main Mar 11, 2024
21 checks passed
@FlorentinDUBOIS FlorentinDUBOIS deleted the binary-access-logs branch March 11, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants