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.

Sign up for GitHub

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

Jump to bottom

[exporter/datadog]: Add container tags to attributes package #6086

Merged
merged 6 commits into from
Nov 5, 2021
Merged

[exporter/datadog]: Add container tags to attributes package #6086

merged 6 commits into from
Nov 5, 2021

Conversation

mackjmr
Copy link
Member

@mackjmr mackjmr commented Nov 1, 2021

Description:
This PR moves the logic for mapping Otel conventions to Datadog conventions for tag _dd.tags.container in the attributes package and adds a more extensive list of container attributes to be mapped.

Link to tracking Issue:

Testing:
Added a couple unit tests.

Documentation:

@mackjmr mackjmr requested a review from mx-psi as a code owner November 1, 2021 18:59
@mackjmr mackjmr requested a review from a team November 1, 2021 18:59
@mackjmr mackjmr force-pushed the datadog_add_container_tags branch from f938986 to 1d99888 Compare November 2, 2021 15:38
@mackjmr mackjmr force-pushed the datadog_add_container_tags branch from 1d99888 to a9f3878 Compare November 2, 2021 15:57
gbbr
gbbr suggested changes Nov 3, 2021
View reviewed changes
Copy link
Member

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, and thanks for adding the test, this is great! I left a few comments.

exporter/datadogexporter/internal/attributes/attributes.go Outdated Show resolved Hide resolved
exporter/datadogexporter/internal/attributes/attributes.go Outdated Show resolved Hide resolved
exporter/datadogexporter/internal/attributes/attributes.go Outdated Show resolved Hide resolved
exporter/datadogexporter/internal/attributes/attributes_test.go Outdated Show resolved Hide resolved
gbbr
gbbr approved these changes Nov 3, 2021
View reviewed changes
Copy link
Member

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Awesome! Much less code! Thanks.

mx-psi
mx-psi requested changes Nov 4, 2021
View reviewed changes
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

@mackjmr you need to fix the lint issues. Running make fmt should work.

mx-psi
mx-psi approved these changes Nov 4, 2021
View reviewed changes
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Looks good. We will also need a backport the attributes changes of this PR to the datadog-agent repository (here), would you mind doing that @mackjmr ?

@mackjmr
Copy link
Member Author

mackjmr commented Nov 4, 2021

Will do!

@mackjmr mackjmr mentioned this pull request Nov 4, 2021
Merged
5 tasks
@mackjmr
Copy link
Member Author

mackjmr commented Nov 4, 2021

Backported the attributes changes in this PR to pkg/otlp/model in the datadog-agent: PR @mx-psi

@bogdandrutu bogdandrutu merged commit e066c9c into open-telemetry:main Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbbr gbbr gbbr approved these changes

@mx-psi mx-psi mx-psi approved these changes

Assignees

@bogdandrutu bogdandrutu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mackjmr @mx-psi @gbbr @bogdandrutu