Log metadata payload as debug to be added to flares #3625
Conversation
checks/collector.py
Outdated
| # to the backend. | ||
| data = payload.payload # deep copy and merge of meta and metric data | ||
| data['apiKey'] = '*************************' + data.get('apiKey', '')[-5:] | ||
| log.debug("Metadata payload: %s", json.dumps(data)) |
There was a problem hiding this comment.
as @gmmeyer pointed out it's probably enough to log this only:
- either when the full host metadata payload is sent: https://github.com/DataDog/dd-agent/blob/5.20.2/checks/collector.py#L644),
- or maybe only on the first run: https://github.com/DataDog/dd-agent/blob/5.20.2/checks/collector.py#L695-L697 logs only the
metaandhost-tagsparts of the metadata payload, but we could log the whole payload if we thing it's useful
There was a problem hiding this comment.
@olivielpeau:
The second one won't work since we will miss external_host_tags. And we could move the log line to _should_send_additional_data but it will result in exactly the same thing except we will be missing the metrics part.
I think we should keep it here to have a payload as close to the one receive by the backend (since the goal is to debug agent/backend interaction).
There was a problem hiding this comment.
Thanks for the explanation @hush-hush!
So if I understand the code correctly this would log the whole agent payload (metadata + metrics + service checks + events) at the end of every run of the collector. Even though it's logged at the debug level I'm still concerned about the size of the log entries this would generate (would be at least a few KBs in general, and up to a few MBs on some agents sending thousands of metrics/events).
I think we should try to select what we think is relevant in the whole payload to debug metadata-related behavior, or at least remove the fields that can be big and are definitely not relevant. Maybe the fields that are defined here: https://github.com/DataDog/dd-agent/blob/5.20.2/checks/collector.py#L63-L66 , and that we could get with payload.meta_payload would be enough? If not could we remove the metrics and service_checks fields (which are not sent in the same payload anyway, cf the splitting done by the emitter), and the events field too (which shouldn't have any impact on the metadata resolution)?
Finally, I think logging this on the first run of the collector should be enough, unless I'm missing something.
Let me know what you think :)
There was a problem hiding this comment.
@olivielpeau: indeed we were going to log it at every collector run... Good catch.
After speaking with the intake team, I think it's important to log the metadata payload as close as possible to what they will receive.
I added a condition to log the payload only on the first run.
c9d264d to
976f714
Compare
When debugging some support case, having the metadata payload in the logs can be very useful.
976f714 to
01c5e17
Compare
What does this PR do?
When debugging some support case, having the metadata payload in the logs
can be very useful.