Add get agent host tags util function #18269
Conversation
|
The |
|
The |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
The |
|
The changelog type |
|
It will be good if we can split this change into 2 PRs, one for |
natashadada
changed the title
| if isinstance(value, list): | ||
| result.extend(value) | ||
| else: | ||
| logger.warning("Unable to use tags for key %s since %s is not a list", key, value) |
There was a problem hiding this comment.
This log and the other warning log are going directly to the customer logs. As a customer, I would have no idea how to interpret this message, which seems more oriented to developers. Can we rework to be more customer-facing?
Co-authored-by: Justin <[email protected]>
| tags_dict = json.loads(host_tags) or {} | ||
| except Exception as e: | ||
| logger.warning("Failed to parse tags: %s", host_tags) | ||
| return result |
There was a problem hiding this comment.
Sorry, one more comment/question here. Is failing silently (with only a log) the expected behavior we want here? I'm trying to think about how this will be used so it's hard to say for certain, but I can see this causing some nasty bugs.
Since this is a base package, the contract should be very clear. Will we ever want to handle this error directly in the integrations?
There was a problem hiding this comment.
Yeah this is a great point. I was going to chat with Zhengda about this tomorrow - I'm not even sure we would hit this case and the experience right now doesn't make sense (would the user have to tell us they see this log and then we'd have to debug it? is it something we could alert on?) I'll talk to Zhengda and Casey tomorrow
What does this PR do?
In DataDog/datadog-agent#28027, I added the get_host_tags function. In this PR, I add a util function that will convert these tags into a list of strings.
Motivation
After version 7.51 of the agent, the resolved_hostname is not set to the agent_hostname. This is because for database monitoring, by default we want the host to be the database host not the agent host. But this means that agent host tags are not being propagated to
postgresql.*metrics. I'd like to use this function from the postgres integration to propagate tags. This PR just adds this function and doesn't call it anywhere. The next PR will update the postgres integration to call it.Additional Notes
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged