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

feat(codecs): Add syslog encoder #21307

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Commits on Mar 15, 2024

  1. feat: Add syslog codec

    Original commit from syedriko
    syedriko authored and polarathene committed Mar 15, 2024
    Configuration menu
    Copy the full SHA
    1ddf6af View commit details
    Browse the repository at this point in the history
  2. chore: Split syslog encoder into separate files

    This is only a temporary change to make the diffs for future commits easier to follow.
    polarathene committed Mar 15, 2024
    Configuration menu
    Copy the full SHA
    7407f7b View commit details
    Browse the repository at this point in the history
  3. refactor: Syslog facility and severity

    - Introduce a `Pri` struct with fields for severity and facility as enum values.
      - `Pri` uses `strum` crate to parse string values into their appropriate enum variant.
      - Handles the responsibility of encoding the two enum values ordinal values into the `PRIVAL` value for the encoder.
    - As `Facility` and `Severity` enums better represent their ordinal mapping directly
      - The `Fixed` + `Field` subtyping with custom deserializer isn't necessary. Parsing a string that represents the enum by name or its ordinal representation is much simpler.
      - Likewise this removes the need for the get methods as the enum can provide both the `String` or `u8` representation as needed.
    polarathene committed Mar 15, 2024
    Configuration menu
    Copy the full SHA
    c9aacd9 View commit details
    Browse the repository at this point in the history
  4. refactor: SyslogSerializer

    `SyslogSerializer::encode()` has been simplified.
    - Only  matching `Event::Log` is relevant, an `if let` bind instead of `match` helps remove a redundant level of nesting.
    - This method only focuses on boilerplate now, delegating the rest to `ConfigDecanter` (_adapt `LogEvent` + encoder config_) and `SyslogMessage` (_encode into syslog message string_).
    - This removes some complexity during actual encoding logic, which should only be concerned about directly encoding from one representation to another, not complimentary features related to Vector config or it's type system.
    
    The new `ConfigDecanter` is where many of the original helper methods that were used by `SyslogSerializer::encode()` now reside. This change better communicates the scope of their usage.
    - Any interaction with `LogEvent` is now contained within the methods of this new struct. Likewise for the consumption of the encoder configuration (instead of queries to config throughout encoding).
    - The `decant_config()` method better illustrates an overview of the data we're encoding and where that's being sourced from via the new `SyslogMessage` struct, which splits off the actual encoding responsibility (see next commit).
    polarathene committed Mar 15, 2024
    Configuration menu
    Copy the full SHA
    d03c87f View commit details
    Browse the repository at this point in the history
  5. refactor: SyslogSerializerConfig

    `SyslogSerializerConfig` has been simplified.
    - Facility / Severity deserializer methods aren't needed, as per their prior refactor with `strum`.
    - The `app_name` default is set via `decant_config()` when not configured explicitly.
    - The other two fields calling a `default_nil_value()` method instead use an option value which encodes `None` into the expected `-` value.
    - Everything else does not need a serde attribute to apply a default, the `Default` trait on the struct is sufficient.
    - `trim_prefix` was removed as it didn't seem relevant. `tag` was also removed as it's represented by several subfields in RFC 5424 which RFC 3164 can also use.
    
    `SyslogMessage::encode()` refactors the original PR encoding logic:
    - Syslog Header fields focused, the PRI and final message value have already been prepared prior. They are only referenced at the end of `encode()` to combine into the final string output.
    - While less efficient than `push_str()`, each match variant has a clear structure returned via the array `join(" ")` which minimizes the noise of `SP` from the original PR. Value preparation prior to this is clear and better documented.
    - `Tag` is a child struct to keep the main logic easy to grok. `StructuredData` is a similar case.
    polarathene committed Mar 15, 2024
    Configuration menu
    Copy the full SHA
    0288a80 View commit details
    Browse the repository at this point in the history
  6. chore: Merge back into syslog.rs

    No changes beyond relocating the code into a single file.
    polarathene committed Mar 15, 2024
    Configuration menu
    Copy the full SHA
    1049ebd View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    3cdc1b4 View commit details
    Browse the repository at this point in the history

Commits on Mar 18, 2024

  1. chore: Housekeeping

    - Drop notes referring to original PR differences + StructuredData adaption references. None of it should be relevant going forward.
    - Revise some other notes.
    - Drop `add_log_source` method (introduced from the original PR author) in favor of using `StructuredData` support instead.
    polarathene committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    3001b67 View commit details
    Browse the repository at this point in the history
  2. chore: DRY into_variant() via akin crate

    This should be simple and lightweight enough to justify for the DRY benefit?
    
    This way the method doesn't need to be duplicated redundantly. That was required because there is no trait for `FromRepr` provided via `strum`. That would require a similar amount of lines for the small duplication here.
    
    The `akin` macro duplicates the `impl` block for each value in the `&enums` array.
    polarathene committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    f8be8d9 View commit details
    Browse the repository at this point in the history

Commits on Mar 20, 2024

  1. chore: Minor revisions

    - `ConfigDecanter::get_message()` replaces the fallback method in favor of `to_string_lossy()` (a dedicated equivalent for converting `Value` type to a String type (_technically it is a CoW str, hence the follow-up with `to_string()`_)).
      - This also encodes the value better, especially for the default `log_namespace: false` as the message value (when `String`) is not quote wrapped, which matches the behaviour of the `text` encoder output.
      - Additionally uses the `LogEvent` method `get_message()` directly from `lib/vector-core/src/event
    /log_event.rs`. This can better retrieve the log message regardless of the `log_namespace` setting.
    - Encoding of RFC 5424 fields has changed to inline the `version` constant directly, instead of via a redundant variable. If there's ever multiple versions that need to be supported, it could be addressed then.
    - The RFC 5424 timestamp has a max precision of microseconds, thus this should be rounded and `AutoSi` can be used (_or `Micros` if it should have fixed padding instead of truncating trailing `000`_).
    polarathene committed Mar 20, 2024
    Configuration menu
    Copy the full SHA
    5360287 View commit details
    Browse the repository at this point in the history
  2. chore: Switch from DateTime<Local> to DateTime<Utc>

    - The original PR author appears to have relied on a hard-coded timestamp key here.
    - `DateTime<Local>` would render the timestamp field with the local timezone offset, but other than that `DateTime<Utc>` would seem more consistent with usage in Vector, especially since any original TZ context is lost by this point?
    - Notes adjusted accordingly, with added TODO query for each encoding mode to potentially support configurable timezone.
    polarathene committed Mar 20, 2024
    Configuration menu
    Copy the full SHA
    38d0d61 View commit details
    Browse the repository at this point in the history

Commits on Apr 1, 2024

  1. chore: Adopt a separate options config struct + minor revisions

    - Move encoder config settings under a single `syslog` config field. This better mirrors configuration options for existing encoders like Avro and CSV.
    - `ConfigDecanter::value_by_key()` appears to accomplish roughly the same as the existing helper method `to_string_lossy()`. Prefer that instead. This also makes the `StructuredData` helper `value_to_string()` redundant too at a glance?
    - Added some reference for the priority value `PRIVAL`.
    - `Pri::from_str_variants()` uses the existing defaults for fallback, communicate that more clearly. Contextual note is no longer useful, removed.
    polarathene committed Apr 1, 2024
    Configuration menu
    Copy the full SHA
    7ef97fb View commit details
    Browse the repository at this point in the history
  2. chore: Switch from String to deserialize Facility + Severity enums

    To better communicate the allowed values, these two config fields can change from the `String` type to their appropriate enum type.
    - This relies on serde to deserialize the config value to the enum which adds a bit more noise to grok.
    - It does make `Pri::from_str_variants()` redundant, while the `into_variant()` methods are refactored to `deserialize()` with a proper error message emitted to match the what serde would normally emit for failed enum variant deserialization.
    - A drawback of this change is that these two config fields lost the ability to reference a different value path in the `LogEvent`. That'll be addressed in a future commit.
    polarathene committed Apr 1, 2024
    Configuration menu
    Copy the full SHA
    34e735d View commit details
    Browse the repository at this point in the history
  3. fix: Support deserializing config value that is a number type

    In a YAML config a string can optionally be wrapped with quotes, while a number that isn't quote wrapped will be treated as a number type.
    
    The current support was only for string numbers, this change now supports flexibility for config using ordinal values in YAML regardless of quote usage.
    
    The previous `Self::into_variant(&s)` logic could have been used instead of bringing in `serde-aux`, but the external helper attribute approach seems easier to grok/follow as the intermediary container still seems required for a terse implementation.
    
    The match statement uses a reference (_which requires a deref for `from_repr`_) to appease the borrow checker for the later borrow needed by `value` in the error message.
    polarathene committed Apr 1, 2024
    Configuration menu
    Copy the full SHA
    2ac3da2 View commit details
    Browse the repository at this point in the history
  4. chore: Add doc comments for enum variants to appease Vector requirement

    This seems redundant given the context? Mostly adds unnecessary noise.
    
    Could probably `impl Configurable` or similar to try workaround the requirement. The metadata description could generate the variant list similar to how it's been handled for error message handling?
    polarathene committed Apr 1, 2024
    Configuration menu
    Copy the full SHA
    7ba64be View commit details
    Browse the repository at this point in the history
  5. chore: Use snafu for error message

    Not sure if this is worthwhile, but it adopts error message convention elsewhere I've seen by managing them via Snafu.
    polarathene committed Apr 1, 2024
    Configuration menu
    Copy the full SHA
    ed202bb View commit details
    Browse the repository at this point in the history