fix(publish-metrics): route debug correctly for publish metrics reporters #2514
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
InesNi
commented
Feb 20, 2024
| @@ -64,11 +64,14 @@ const vendorTranslators = { | |||
| return otelTemplate(config, dynatraceTraceSettings); | |||
| }, | |||
| 'open-telemetry': (config) => { | |||
| let tracesConfig = config; | |||
| let newConfig = config; | |||
There was a problem hiding this comment.
Adding the type to metrics block here to make the reporter name available for the metric pipeline.
Note: Changing name to newConfig as makes more sense, especially now when adding the metrics check as well.
InesNi
commented
Feb 20, 2024
| @@ -24,7 +23,7 @@ context.setGlobalContextManager(contextManager); | |||
| // DEBUGGING SETUP - setting the OpenTelemetry's internal diagnostic handler here to run when debug is enabled | |||
| if ( | |||
| process.env.DEBUG && | |||
| process.env.DEBUG === 'plugin:publish-metrics:open-telemetry' | |||
| process.env.DEBUG.includes('plugin:publish-metrics:') | |||
There was a problem hiding this comment.
Setting OTels internal logger to run when DEBUG is set for any reporter, not just open-telemetry
InesNi
commented
Feb 20, 2024
| } | ||
| } | ||
|
|
||
| class OTelTraceBase { | ||
| constructor(config, script) { | ||
| this.config = config; | ||
| this.script = script; | ||
| this.debug = require('debug')(`plugin:publish-metrics:${this.config.type}`); |
There was a problem hiding this comment.
Setting this.debug here with reporter type will make it available to both HTTP and Playwright trace reporters.
InesNi
commented
Feb 20, 2024
| @@ -45,6 +44,7 @@ class OTelMetricsReporter { | |||
| } | |||
|
|
|||
| configure(config) { | |||
| this.debug = require('debug')(`plugin:publish-metrics:${this.config.type}`); | |||
There was a problem hiding this comment.
Currently OTel reporter metrics doesn't support any vendor-specific reporters, but for future proofing we require dynamically.
bernardobridge
approved these changes
Feb 21, 2024
InesNi
deleted the
ifazlic-art-1655-route-debug-correctly-for-publish-metrics-reporters
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When
DEBUGis set to a vendor-specific reporter (frompublish-metrics) if that reporter is using OTel in the background we want to log OTel reporters debug logs as well.e.g. When user uses the Datadog reporter for tracing and sets
DEBUG=plugin:publish-metrics:datadog, we want to log debug logs from OTel reporter as well as OTel is the one running the tracing.Implementation
DEBUGis set to any reporter, not justopen-telemetrydebugwill be dynamically set for metrics and traces, using thetypeproperty from config set to the reporters name.In config translator functions
typeis set on bothtracesandmetricspart of config so that classes responsible for metrics and traces can set the debug accordingly.Testing
Tested manually, running multiple reporters and types of errors to trigger all debug logs and make sure the output is correct.
Pre-merge checklist