Skip to content

Allow for Enabling/Disabling KVP using Config #179

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

Merged
merged 5 commits into from
Mar 24, 2025

Conversation

peytonr18
Copy link
Contributor

This PR introduces the ability to enable or disable KVP logging through the telemetry.kvp_diagnostics field in the configuration.

Changes

  • The setup_layers function now checks config.telemetry.kvp_diagnostics before initializing emit_kvp_layer.
  • If kvp_diagnostics = false, emit_kvp_layer is set to None, ensuring no logs are written to the EmitKVPStruct.
  • If config is None, the default behavior (true) is maintained.
  • Added a unit test in kvp.rs to validate that no data is written when KVP is disabled.

Key Point: Config Tracing & KVP Behavior

  • Before the config is loaded, setup_layers is called once with None, meaning the config values will still be written to KVP. If kvp_diagnostics = false, KVP logging will stop after the second call to setup_layers, since the new subscriber overrides the previous one.

Dependency on probertson-cache-vm-id-logging-new

This PR is based on probertson-cache-vm-id-logging-new, as that branch introduces passing Config into setup_layers, which is required for this logic.

Next Steps

  • Merge probertson-cache-vm-id-logging-new first.
  • Then merge this PR to introduce configurable KVP diagnostics.

Resolves #152.

@peytonr18 peytonr18 marked this pull request as ready for review March 18, 2025 21:02
@peytonr18 peytonr18 force-pushed the probertson-disable-kvp branch from ec4cd50 to bc07e75 Compare March 18, 2025 21:06
@peytonr18 peytonr18 force-pushed the probertson-disable-kvp branch from bc07e75 to 91ec0e4 Compare March 19, 2025 00:22
@peytonr18 peytonr18 merged commit 5e83a46 into Azure:main Mar 24, 2025
5 checks passed
@peytonr18 peytonr18 deleted the probertson-disable-kvp branch March 27, 2025 21:16
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.

Allow for Enabling/Disabling KVP using Config Struct
4 participants