Skip to content
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

Send device as a resource and not a tag #16264

Merged
merged 5 commits into from
Mar 29, 2023
Merged

Send device as a resource and not a tag #16264

merged 5 commits into from
Mar 29, 2023

Conversation

vickenty
Copy link
Contributor

@vickenty vickenty commented Mar 23, 2023

What does this PR do?

Add a missing call to pull device out of tags into the resources section.

This fixes a capitalization issue when device tag was not available in upper-case when sending metrics via v2 API (a special case compared to other tags).

Motivation

Additional Notes

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Create a device with an uppercase name and mount it:

dd if=/dev/zero of=image
sudo losetup /dev/loop0 image
sudo ln /dev/loop0 /dev/CASE
mke2fs /dev/CASE
mount /dev/CASE /mnt

Run the agent with the disk check enabled (usually enabled by default).

Check that system.disk.free and other system.disk metrics have two device tags: one lowercased, one as is.

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Use the major_change label if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.
  • A release note has been added or the changelog/no-changelog label has been applied.
  • Changed code has automated tests for its functionality.
  • Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
  • At least one team/.. label has been applied, indicating the team(s) that should QA this change.
  • If applicable, docs team has been notified or an issue has been opened on the documentation repo.
  • If applicable, the need-change/operator and need-change/helm labels have been applied.
  • If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
  • If applicable, the config template has been updated.

@vickenty vickenty added this to the 7.45.0 milestone Mar 23, 2023
Add a missing call to pull `device` out of tags into the resources
section.

This fixes a capitalization issue when device tag was not available in
upper-case when sending metrics via v2 API (a special case compared to
other tags).
Instead of pre-filling Device field, pass the device as a tag and make
sure that the serialization code extracts the device information from
the tag.
@vickenty vickenty marked this pull request as ready for review March 27, 2023 13:31
@vickenty vickenty requested review from a team as code owners March 27, 2023 13:31
Copy link
Contributor

@maycmlee maycmlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small edit

releasenotes/notes/device-620eb9d825e0a1ea.yaml Outdated Show resolved Hide resolved
@vickenty vickenty merged commit 56a690d into main Mar 29, 2023
@vickenty vickenty deleted the vickenty/dev branch March 29, 2023 09:07
@rpriyanshu9
Copy link
Contributor

@vickenty @maycmlee @gh123man - I've upgraded the datadog agent to 7.45.0 - and I'm no longer able to get the device tag for the system.disk metrics. It seems that device tag has changed to resource.device tag. Is that a result of this PR?

@vickenty
Copy link
Contributor Author

@rpriyanshu9 @datsabk This PR changes how the device information is reported to the backend, but the tags you see on the metrics in the app should be the same. If you have any issues, please reach out to Datadog support.

@rpriyanshu9
Copy link
Contributor

Thanks for the confirmation. I've opened up a datadog support ticket to report this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants