Add more deprecation logs and mute by default#40305
Conversation
| OTEL_EXPORTER_OTLP_ENDPOINT = "https://otlp.signoz.dogfood.fleetdm.com" | ||
| # FLEET_LOGGING_TRACING_ENABLED = "true" | ||
| # FLEET_LOGGING_TRACING_TYPE = "elasticapm" | ||
| FLEET_LOGGING_ENABLE_TOPICS = "deprecated-field-names" |
There was a problem hiding this comment.
@rfairburn I think this is the right place to add an env var for our Dogfood server, let me know if there's a different place I need to change. This will turn on deprecation warnings on Dogfood so we can see them. The warnings will be turned off by default for the upcoming release.
| // key names back to old (json tag) names so that structs can be unmarshaled | ||
| // correctly when input uses the new key names. | ||
| func rewriteNewToOldKeys(raw json.RawMessage, target any) json.RawMessage { | ||
| func rewriteNewToOldKeys(raw json.RawMessage, target any) (json.RawMessage, map[string]string, error) { |
There was a problem hiding this comment.
Updated to return the set of deprecated keys used, if any, for use in warning logs.
|
|
||
| // GroupFromBytes parses a Group from concatenated YAML specs. | ||
| func GroupFromBytes(b []byte) (*Group, error) { | ||
| func GroupFromBytes(b []byte, options ...GroupFromBytesOpts) (*Group, error) { |
There was a problem hiding this comment.
Added optional param so that you can send a logger in to GroupFromBytes, which is a more centralized place to log deprecation warnings. Passing the deprecated keys back to the caller would mean having to implement the same logs in several places.
rfairburn
left a comment
There was a problem hiding this comment.
Terraform change looks fine
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #40305 +/- ##
==========================================
+ Coverage 66.28% 66.37% +0.09%
==========================================
Files 2459 2456 -3
Lines 196811 196815 +4
Branches 8714 8577 -137
==========================================
+ Hits 130451 130643 +192
+ Misses 54542 54349 -193
- Partials 11818 11823 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
iansltx
left a comment
There was a problem hiding this comment.
Minor feedback/questions here. Quessing the reason for having to gate topics this way is we have a number of cases where we're using a very basic logFn rather than slog? Are there any places where the built-in topic handling actually benefits us?
|
Updated to use |
|
@rfairburn can you re-👍 when you have a sec? (no changes to the terraform stuff) |
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #40122 # Details * Adds deprecation warnings to `fleetctl apply` * Adds alias conflict errors (i.e. using both new and deprecated keys in the same spec) to `fleetctl apply` * Adds logic around all deprecated field warnings to check the topic first * Disables deprecation warnings by default for `fleet serve`, `fleetctl gitops` and `fleetctl apply` * Enables deprecation warnings for dogfood via env var To turn on warnings: * In `fleet serve`, use either `--logging_enable_topics=deprecated-field-names` or the `FLEET_LOGGING_ENABLE_TOPICS=deprecated-field-names` env var * In `fleetctl gitops` / `fleetctl apply` use either `--enable-log-topics=deprecated-field-names` or `FLEET_ENABLE_LOG_TOPICS=deprecated-field-names` # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually tested in `fleetctl apply`, `fleet serve` and `fleet gitops` that warnings are suppressed by default and added when the appropriate env var or CLI option is used (cherry picked from commit 772fb12)
Related issue: Resolves #40122
Details
fleetctl applyfleetctl applyfleet serve,fleetctl gitopsandfleetctl applyTo turn on warnings:
fleet serve, use either--logging_enable_topics=deprecated-field-namesor theFLEET_LOGGING_ENABLE_TOPICS=deprecated-field-namesenv varfleetctl gitops/fleetctl applyuse either--enable-log-topics=deprecated-field-namesorFLEET_ENABLE_LOG_TOPICS=deprecated-field-namesChecklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
tested in
fleetctl apply,fleet serveandfleet gitopsthat warnings are suppressed by default and added when the appropriate env var or CLI option is used