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

Add Retries to gRPC version of OTLP Exporter #3883

Closed
wants to merge 15 commits into from
Closed

Add Retries to gRPC version of OTLP Exporter #3883

wants to merge 15 commits into from

Conversation

mic-max
Copy link
Contributor

@mic-max mic-max commented Nov 9, 2022

Towards #1779

Changes

Enables retry for OTLP Exporter gRPC by default.

The default values used to initialize Grpc.Net.Client.Configuration.RetryPolicy were taken from the Java SIG's implementation.

  • RetryMaxAttempts = 5

  • RetryInitialBackoff = 1 second

  • RetryMaxBackoff = 5 seconds

  • RetryBackoffMultiplier = 1.5x

  • RetryableStatusCodes taken from specification of which are retryable

  • Added several unit tests with new mock OTLP collector thanks to @alanwest for creating it, letting me know and helping me onboard to it.

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@mic-max mic-max marked this pull request as ready for review November 16, 2022 21:54
@mic-max mic-max requested a review from a team November 16, 2022 21:54
@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #3883 (86aa870) into main (bf95a54) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 86aa870 differs from pull request most recent head a5fac98. Consider uploading reports for the commit a5fac98 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3883      +/-   ##
==========================================
- Coverage   87.40%   87.40%   -0.01%     
==========================================
  Files         278      278              
  Lines       10757    10786      +29     
==========================================
+ Hits         9402     9427      +25     
- Misses       1355     1359       +4     
Impacted Files Coverage Δ
...TelemetryProtocol/OtlpExporterOptionsExtensions.cs 97.52% <100.00%> (+0.78%) ⬆️
...porter.OpenTelemetryProtocol/OtlpMetricExporter.cs 72.72% <0.00%> (-13.64%) ⬇️
...tation/OpenTelemetryProtocolExporterEventSource.cs 85.00% <0.00%> (-10.00%) ⬇️
...ZPages/Implementation/ZPagesExporterEventSource.cs 56.25% <0.00%> (-6.25%) ⬇️
src/OpenTelemetry/BatchExportProcessor.cs 84.11% <0.00%> (+1.86%) ⬆️

{
StatusCode.Cancelled,
StatusCode.DeadlineExceeded,
StatusCode.ResourceExhausted, // TODO: Investigate this one.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

My initial assumption is that the gRPC client handles this. Can probably validate the behavior conforms with the specs requirements with the mock server.

Copy link
Member

Choose a reason for hiding this comment

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

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Nov 26, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Issues and pull requests which have been flagged for closing due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants