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

Handle partial success responses in the OTLP HTTP exporter #6970

Merged
merged 5 commits into from
Jun 27, 2023
Merged

Handle partial success responses in the OTLP HTTP exporter #6970

merged 5 commits into from
Jun 27, 2023

Conversation

evan-bradley
Copy link
Contributor

Description:

Handle partial success messages returned from OTLP HTTP backends.

Link to tracking Issue:

Fixes #6686

Testing:

Unit tests. I was able to manually test partial success messages when submitting metrics to a vendor backend. I'd welcome any additional places to test this.

@evan-bradley evan-bradley requested review from a team and djaglowski January 19, 2023 00:26
@codecov
Copy link

codecov bot commented Jan 19, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.24 ⚠️

Comparison is base (45d9a9e) 91.14% compared to head (58e8605) 90.90%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6970      +/-   ##
==========================================
- Coverage   91.14%   90.90%   -0.24%     
==========================================
  Files         298      300       +2     
  Lines       14915    15082     +167     
==========================================
+ Hits        13594    13711     +117     
- Misses       1045     1097      +52     
+ Partials      276      274       -2
Impacted Files Coverage Δ
exporter/otlphttpexporter/otlp.go 91.97% <100.00%> (+12.45%) ⬆️

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@evan-bradley evan-bradley force-pushed the otlphttp-partial-success branch 2 times, most recently from 5ae21fb to 876ab4b Compare January 19, 2023 17:26
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 7, 2023
@github-actions github-actions bot removed the Stale label Feb 8, 2023
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 22, 2023
djaglowski
djaglowski approved these changes Feb 22, 2023
View reviewed changes
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

LGTM

@djaglowski
Copy link
Member

I think we need a rebase in order to pass the mandatory 1.20 check.

@github-actions github-actions bot removed the Stale label Feb 23, 2023
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Mar 18, 2023
bogdandrutu
bogdandrutu reviewed Mar 21, 2023
View reviewed changes
exporter/otlphttpexporter/otlp_test.go Outdated Show resolved Hide resolved
exporter/otlphttpexporter/otlp_test.go Outdated Show resolved Hide resolved
exporter/otlphttpexporter/otlp.go Outdated Show resolved Hide resolved
exporter/otlphttpexporter/otlp.go Show resolved Hide resolved
@github-actions github-actions bot removed the Stale label Mar 22, 2023
@evan-bradley evan-bradley force-pushed the otlphttp-partial-success branch 2 times, most recently from 14bf1ab to a97b2a6 Compare March 31, 2023 14:34
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Apr 20, 2023
@github-actions github-actions bot removed the Stale label Apr 22, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 7, 2023
@github-actions github-actions bot removed the Stale label May 10, 2023
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 31, 2023
@evan-bradley
Copy link
Contributor Author

@dmitryax Could you please take a look at this one as well? I believe this PR is also ready to go.

@github-actions github-actions bot removed the Stale label Jun 3, 2023
dmitryax
dmitryax approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

LGTM. Just one minor suggesting

exporter/otlphttpexporter/otlp.go Outdated Show resolved Hide resolved
@PhilWillMill
Copy link

LGTM? We're currently hitting partial success responses and this will definitely help us investigate

@dmitryax dmitryax merged commit 5852d09 into open-telemetry:main Jun 27, 2023
29 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 27, 2023
@jmacd jmacd mentioned this pull request Jun 28, 2023
Closed
@breedx-splk breedx-splk mentioned this pull request Aug 23, 2023
Closed
@evan-bradley evan-bradley mentioned this pull request Jun 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joaopgrassi joaopgrassi joaopgrassi left review comments

@djaglowski djaglowski djaglowski approved these changes

@dmitryax dmitryax dmitryax approved these changes

@bogdandrutu bogdandrutu Awaiting requested review from bogdandrutu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.81.0
Development

Successfully merging this pull request may close these issues.

Handle partial success responses from OTLP export services
6 participants
@evan-bradley @djaglowski @PhilWillMill @bogdandrutu @joaopgrassi @dmitryax