Skip to content

Add a notion of Request to otlp to decouple data from protocol specific metadata #4050

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
Merged
merged 1 commit into from
Sep 16, 2021
Merged

Add a notion of Request to otlp to decouple data from protocol specific metadata #4050

merged 1 commit into from
Sep 16, 2021

Conversation

bogdandrutu
Copy link
Member

Signed-off-by: Bogdan Drutu [email protected]

@bogdandrutu bogdandrutu requested review from a team and jpkrohling September 15, 2021 16:04
codeboten
codeboten reviewed Sep 15, 2021
View reviewed changes
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Can you give me a bit more context around the metadata you want to decouple with this change?

@bogdandrutu
Copy link
Member Author

@codeboten gave that in the Collector SIG. See also open-telemetry/opentelemetry-proto#332

@bogdandrutu bogdandrutu force-pushed the request branch 2 times, most recently from e0dcf21 to 1d1b7b8 Compare September 15, 2021 17:22
codeboten
codeboten reviewed Sep 15, 2021
View reviewed changes
Copy link
Contributor

@codeboten codeboten 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 addressing my comments, looks like the build is still failing.

@jpkrohling
Copy link
Member

I'm trying to assess what does this mean for the current pdata we have. Would consumers of pdata.NewTraces be affected? Or would they just have another option?

@bogdandrutu
Copy link
Member Author

Only the consumer of Marshalers will be affected since the Marshaler of pdata.Traces will be the Traces message that we define in the proto, to marshal to the request you need to construct the TracesRequest

jpkrohling
jpkrohling approved these changes Sep 16, 2021
View reviewed changes
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

LGTM

@bogdandrutu bogdandrutu merged commit f3cd422 into open-telemetry:main Sep 16, 2021
@bogdandrutu bogdandrutu deleted the request branch September 16, 2021 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeboten codeboten codeboten left review comments

@jpkrohling jpkrohling jpkrohling approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bogdandrutu @jpkrohling @codeboten