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

pkg/trace/api: OTLP: match datadogexporter behaviour #11548

Merged
merged 3 commits into from
Apr 28, 2022
Merged

pkg/trace/api: OTLP: match datadogexporter behaviour #11548

merged 3 commits into from
Apr 28, 2022

Conversation

gbbr
Copy link
Contributor

@gbbr gbbr commented Apr 4, 2022 •
edited
Loading

This change refactors the OTLP receiver to be compatible with the collector's datadogexporter and attempts matching all behaviours, as well as adding more tests.

@gbbr gbbr requested review from a team as code owners April 4, 2022 11:13
@gbbr gbbr force-pushed the gbbr/otlp-patch branch from fe7f377 to 99e7ece Compare April 4, 2022 11:15
mx-psi
mx-psi requested changes Apr 4, 2022
View reviewed changes
pkg/trace/api/otlp.go Show resolved Hide resolved
pkg/trace/api/otlp.go Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
@gbbr gbbr requested a review from mx-psi April 4, 2022 11:45
@gbbr gbbr force-pushed the gbbr/otlp-patch branch from a169cfe to c05e9a7 Compare April 4, 2022 11:47
@gbbr gbbr added the team/agent-apm trace-agent label Apr 4, 2022
@gbbr gbbr added this to the 7.36.0 milestone Apr 4, 2022
@gbbr gbbr force-pushed the gbbr/otlp-patch branch from c05e9a7 to 1b25357 Compare April 4, 2022 12:18
mx-psi
mx-psi approved these changes Apr 4, 2022
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.

LGTM, but I would still want someone from Agent APM to review this since this is not my area of expertise

@gbbr gbbr force-pushed the gbbr/otlp-patch branch from 1b25357 to 902b5be Compare April 4, 2022 12:24
maycmlee
maycmlee reviewed Apr 4, 2022
View reviewed changes
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.

Just a small edit

releasenotes/notes/apm-otlp-standardize-attributes-7a52f9ccbf3e9152.yaml Outdated Show resolved Hide resolved
ajgajg1134
ajgajg1134 approved these changes Apr 4, 2022
View reviewed changes
Copy link
Contributor

@ajgajg1134 ajgajg1134 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 to me!

@gbbr gbbr force-pushed the gbbr/otlp-patch branch from 902b5be to 702e29c Compare April 5, 2022 06:08
@kacper-murzyn kacper-murzyn modified the milestones: 7.36.0, 7.37.0 Apr 5, 2022
@gbbr gbbr force-pushed the gbbr/otlp-patch branch 6 times, most recently from 5b421e3 to d537caa Compare April 8, 2022 06:41
@gbbr gbbr requested a review from a team as a code owner April 8, 2022 07:36
@gbbr gbbr changed the title pkg/trace/api: OTLP: ensure attribute keys get standardised to Datadog tags pkg/trace/api: OTLP: match datadogexporter behaviour Apr 8, 2022
djmitche
djmitche approved these changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

pkg/config looks good

@gbbr gbbr force-pushed the gbbr/otlp-patch branch 2 times, most recently from 68ac5c4 to f1abb10 Compare April 14, 2022 07:50
@gbbr gbbr force-pushed the gbbr/otlp-patch branch 8 times, most recently from 57e3a01 to b5a64ce Compare April 26, 2022 08:22
@gbbr gbbr force-pushed the gbbr/otlp-patch branch 2 times, most recently from ad3fcf1 to c8771a4 Compare April 26, 2022 10:12
@gbbr gbbr requested a review from mx-psi April 26, 2022 10:12
@gbbr gbbr force-pushed the gbbr/otlp-patch branch 2 times, most recently from 84c2b24 to aca042f Compare April 26, 2022 11:01
mx-psi
mx-psi reviewed Apr 27, 2022
View reviewed changes
pkg/config/config_template.yaml Outdated Show resolved Hide resolved
pkg/config/config_template.yaml Outdated Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
pkg/trace/go.mod Show resolved Hide resolved
@gbbr gbbr requested a review from a team as a code owner April 28, 2022 07:11
gbbr
gbbr commented Apr 28, 2022
View reviewed changes
Copy link
Contributor Author

@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.

Thank you for looking. I think I covered everything.

pkg/config/config_template.yaml Outdated Show resolved Hide resolved
pkg/config/config_template.yaml Outdated Show resolved Hide resolved
pkg/trace/go.mod Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
pkg/trace/api/otlp.go Outdated Show resolved Hide resolved
@gbbr gbbr requested a review from mx-psi April 28, 2022 07:11
mx-psi
mx-psi approved these changes Apr 28, 2022
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.

LGTM. I don't know enough about the Datadog traces exporter to understand if there will be a 100% match once the code is replaced there, but otherwise everything makes sense AFAICT

@gbbr gbbr force-pushed the gbbr/otlp-patch branch 2 times, most recently from f3481f0 to 12154b3 Compare April 28, 2022 09:04
@gbbr gbbr force-pushed the gbbr/otlp-patch branch from 12154b3 to 40aeca9 Compare April 28, 2022 09:15
@gbbr gbbr merged commit 49ece7f into main Apr 28, 2022
@gbbr gbbr deleted the gbbr/otlp-patch branch April 28, 2022 10:24
@gbbr gbbr mentioned this pull request May 9, 2022
Merged
@dragon3 dragon3 mentioned this pull request May 27, 2022
Closed
9 tasks
andrewlock added a commit that referenced this pull request Mar 23, 2023
@andrewlock
Allow trace UDS/named pipe transports when TCP disabled
0f0ab7e 
Previously when `apm_config.receiver_port: 0`, we would not try using any transports (UDS/windows named pipes). This change was introduced in #11548.

In this PR we now only bail out if all the transports are disabled.
@andrewlock andrewlock mentioned this pull request Mar 23, 2023
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maycmlee maycmlee maycmlee left review comments

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

@ajgajg1134 ajgajg1134 ajgajg1134 approved these changes

@djmitche djmitche djmitche approved these changes

Assignees
No one assigned
Labels
7.36.0-drop team/agent-apm trace-agent
Projects
None yet
Milestone
7.37.0
Development

Successfully merging this pull request may close these issues.
6 participants
@gbbr @mx-psi @djmitche @ajgajg1134 @maycmlee @kacper-murzyn