[Exporter/Datadog] Added debug flag to print metrics payload #7570
Conversation
|
|
ikhan-tc
changed the title
|
@mx-psi Can you please review this PR. I added back that line of code which is spamming the logs but changed the log level to "debug" instead of "info". |
| @@ -178,6 +178,7 @@ func (exp *metricsExporter) PushMetricsData(ctx context.Context, md pdata.Metric | |||
|
|
|||
| err = nil | |||
| if len(ms) > 0 { | |||
| exp.params.Logger.Debug("exporting payload", zap.Any("metric", ms)) | |||
There was a problem hiding this comment.
Indentation here looks odd. Did you run gofmt and/or the linter target? This will likely fail the CI.
There was a problem hiding this comment.
@jpkrohling Can you please review this PR again. Tested with gofmt and linter this time.
ikhan-tc
force-pushed
the
add_debug_log_payload
branch
from
81adb42 to
1126067
Compare
There was a problem hiding this comment.
Thanks for your contribution @ikhan-tc and apologies for the delay in replying.
I understand the need for this and this has been an ask by other users before. However, these are not the only types of payloads that we export for metrics, and it would be confusing to log only some of them. In particular:
- We export 'sketches' payloads for Histogram metrics by default here these are sent in Protobuf format, so logging them would be a bit more difficult. Their format is also a bit more difficult to understand.
- We export 'host metadata' payloads here . These are also relevant for metrics; e.g. the
tagsfield in the configuration is sent in this payload together with the host and then the backend applies these tags to the metrics. We already log this one.
If we only log the 'metrics series' payloads, users that are exporting Histograms could be confused). I think we should at least log something for sketches: at the very least number of sketches exported (ideally also their names).
A second question is whether we want these to be logged unconditionally in debug mode. I am undecided on this one; what do you think about this? I would try to look at what other exporters do over the next few days to help decide on this.
|
Hey again, I had a look at what other exporters. I think from my side I am okay to log unconditionally on debug, but to accept this PR I want to log also the sketches payloads then. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
@kmotupalli-tc Thanks! I left a comment, code looks good now :) |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
#8929 was merged, so I am closing this. If this needs to be reopened for any reason please ping me! |
Description: Added debug logging to log metrics payload data sent to Datadog. Redo #5856 as debug log instead of Info log. Metrics payload data will be logged if collector is started in debug mode.
Link to tracking Issue: #7528
Testing: Started the collector in debug mode to test the output. The metrics payload data only logged if the collector is in debug mode.