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

logging: consider moving away from cluster settings and into logging filters #57878

Open
knz opened this issue Dec 13, 2020 · 9 comments
Open
Labels
A-cc-enablement Pertains to current CC production issues or short-term projects A-logging In and around the logging infrastructure. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-observability

Comments

@knz
Copy link
Contributor

knz commented Dec 13, 2020

Today we have certain notable events for which we use a cluster setting to enable/disable the logging. This includes:

These logging events are only logged conditionally because they are otherwise rather frequent, and we did not want them to "pollute" logs or incur excessive disk IOPS when logging to files.

However, this is inconvenient for CC logging, where we likely do want to see these events collected and instead filter them "elsewhere" (in a network log collector). For CC clusters, this is extra inconvenient because it requires some additional cluster settings to be set before a CC cluster becomes operational, which is error-prone (both due to human mistakes and additional software errors).

Instead, we could consider have the internal CockroachDB components log them unconditionally, and have a single general-purpose filtering option in the logging system:

  • when logging to files exclusively, the filter would be pre-set to exclude these event types
  • when logging to the network, the filter would not be pre-set so all events are routed to the network.

The filter could be like a list of logging events to exclude, per sink. For example in YAML:

sinks:
  file-groups:
    default:
      exclude-events: create_statistics, client_connection_start, client_connection_end

This additional mechanism would also make it possible for end-users to opt in and out of logging certain notable events without requiring us to add cluster settings for each of them every time.

@aaron-crl thoughts? @thtruo ?

Jira issue: CRDB-3472

@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-logging In and around the logging infrastructure. labels Dec 13, 2020
@knz
Copy link
Contributor Author

knz commented Dec 13, 2020

cc @logston I think you'll want to learn this

@knz knz changed the title logging: consider moving away from cluster settings and into logging filter logging: consider moving away from cluster settings and into logging filters Dec 13, 2020
@knz
Copy link
Contributor Author

knz commented Dec 15, 2020

In fact as per @tbg we may need to do so urgently, because in multi-tenant individual tenants cannot set cluster settings. So if we want these logs they can't be controlled by cluster settings for now.

@tbg
Copy link
Member

tbg commented Dec 15, 2020

What we can always do is override individual cluster settings from the env in start-sql.

@knz
Copy link
Contributor Author

knz commented Dec 15, 2020

What we can always do is override individual cluster settings from the env in start-sql.

Is this override mechanism already implemented? If so, where can I find more information about it?

@tbg
Copy link
Member

tbg commented Dec 15, 2020

Settings have a .Override method, which is what I was referring to.
This isn't the right option if we start letting tenants listen to their settings table, of course. Then, nothing else would be required.

@knz
Copy link
Contributor Author

knz commented Dec 15, 2020

ok so if nothing else we can implement a new override mechanism in the mt start-sql CLI flag handling. That'd be a temporary workaround, but it doesn't hurt.

@aaron-crl
Copy link

Logging filters sound like a good strategy. That could also potentially allow end users to tune it such that events we don't traditionally think of as security events go to a custom sink for their own consumption.

@thtruo
Copy link
Contributor

thtruo commented Jan 14, 2021

Echoing Aaron's sentiment, moving towards logging filters and away from setting clusters sounds like a great approach. The usability improvement around managing logs in a single place (at log configuration) instead of managing knobs at multiple places will lead to a quality of life improvement for users. I also like your idea of an exclusion list too because it gives the end user more flexibility and control around what events go where.

@jlinder jlinder added the T-server-and-security DB Server & Security label Jun 16, 2021
@knz knz added the A-cc-enablement Pertains to current CC production issues or short-term projects label Jul 29, 2021
@thtruo thtruo added T-observability-inf and removed T-server-and-security DB Server & Security labels Feb 18, 2022
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cc-enablement Pertains to current CC production issues or short-term projects A-logging In and around the logging infrastructure. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-observability
Projects
None yet
Development

No branches or pull requests

5 participants