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

Adding structured logs to syslog #501

Closed

Conversation

rafaelwestphal
Copy link
Contributor

Replacing the tail plugin with systemd plugin gives ops-agent structured
logs when reading /var/log/syslog content.

@quentinmit
Copy link
Member

#256 is a blocker for this IMO.

@qingling128
Copy link
Contributor

Do you mind attaching the before & after log entries examples to the PR description?

BTW, for the impact of existing users, is the expectation like below?

Case 1 - Users who don't adjust the default syslog today

Impact: The log entry would change from unstructured to structured.

Case 2 - Users who customize the included_paths today for the default syslog

logging:
  receivers:
    syslog:
      type: files
      include_paths:
      - /var/log/messages // For distros that has both `/var/log/messages` and `/var/log/syslog` files (b/206027564), users might customize this.

Impact: No change since it will stay overwritten by their custom config.

Case 3 - Users have customized the default syslog pipeline to have additional processors (e.g. parse_regex) chained with the default syslog receiver

logging:
  processors:
    parse:
      type: parse_regex
      ......
  service:
    pipelines:
      default_pipeline:
        receivers: [syslog]
        processors: [parse]

Impact: Their original parser would no longer take effect I assume. If the receiver changed to systemd_journald under the hood, what's the behavior of the parse_regex processor in this case? I guess it's a no-op instead of dropping the logs if failed to parse it through the expected regex.

@jefferbrecht
Copy link
Member

jefferbrecht commented Mar 31, 2022

There's also the issue of message being renamed to MESSAGE. This can break users in cases 1 and 3. For case 3 in particular, even if they update the regex it'll still behave unexpectedly until they also add field: MESSAGE.

Case 1 is more of a soft breakage since there are no processors, but they could still have queries set up in Cloud Logging that depend on jsonPayload.message, and AFAICT those are case-sensitive.

This could be solved by adding a mapping from MESSAGE to message in all journald receivers (requested in #256), but that'd be a breaking change too for anyone relying on it being MESSAGE...

Brainstorming here but what about adding a message_field configuration to systemd_journald that defaults to MESSAGE but we set it to message for the built-in syslog receiver? It would be weirdly inconsistent but AFAICT it shouldn't be a breaking change.

Replacing the tail plugin with systemd plugin gives ops-agent structured
logs when reading /var/log/syslog content.
@rafaelwestphal rafaelwestphal force-pushed the westphalrafael-replace-tail-for-syslog branch from de62282 to 64b62bc Compare April 1, 2022 15:27
@qingling128
Copy link
Contributor

qingling128 commented Apr 1, 2022

If it's infeasible to keep the 3 cases above non-breaking, another alternative is to leave the built-in config untouched, and document the config users can drop in in order to get structured syslog, and explain that any processors that are chained with the syslog receiver needs to be adjusted accordingly. This way it's up to the users to decide whether they want to adopt this change or stick with the default behavior because of some downstream processors that depend on the files receiver:

logging:
  receivers:
    syslog:
      type: systemd_journald

@qingling128
Copy link
Contributor

Per discussion offline, we will leave the build-in config untouched, and follow up with a documentation change instead to let users know how to collect structured system logs before the next breaking change. See the tracker for the documentation change at b/229867178

@ridwanmsharif ridwanmsharif deleted the westphalrafael-replace-tail-for-syslog branch July 23, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants