-
Notifications
You must be signed in to change notification settings - Fork 848
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 http/protobuf retry #3983
Add http/protobuf retry #3983
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3983 +/- ##
============================================
+ Coverage 89.73% 89.86% +0.13%
- Complexity 4271 4282 +11
============================================
Files 512 513 +1
Lines 12941 12960 +19
Branches 1249 1251 +2
============================================
+ Hits 11612 11646 +34
+ Misses 925 912 -13
+ Partials 404 402 -2
Continue to review full report at Codecov.
|
@@ -10,8 +10,7 @@ otelJava.moduleName.set("io.opentelemetry.exporter.otlp.http.logs") | |||
|
|||
dependencies { | |||
api(project(":sdk:logs")) | |||
|
|||
implementation(project(":exporters:otlp:common")) | |||
api(project(":exporters:otlp:common")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think sticking with implementation is better, even if it means duplicating into some build files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We talked about this back here, but I've tested it and confirmed that we can get away with implementation.
Best to keep it as implementation until we actually promote something in common out of internal. Will update the grpc modules as well.
} | ||
} | ||
|
||
private final HeldCertificate heldCertificate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're extracting this, you could consider switching back to SelfSignedCertificate
from armeria as the incompatibility with okhttp was fixed
io.opentelemetry.exporter.otlp.internal.retry
package relocate retry related classesIt wasn't entirely clear which http status codes were retryable, so I opened an issue with the spec. Assumed the following are retryable: 429, 502, 503, 504.