-
Notifications
You must be signed in to change notification settings - Fork 1k
Further enhancement to OtlpMetricsSender #6025
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
Conversation
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 may tweak things more, but I wanted to open this for review.
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.
Having this (as package-private) made things more complicated when introducing the Request class that we need to be public, so I removed it. It was only there to demonstrate that the OtlpMetricsSender could be made more generic for use in other registries, but until we verify that need more, let's remove this.
response = httpRequest.send(); | ||
} | ||
catch (Throwable e) { | ||
throw new Exception(e); |
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.
Since the HttpSender
throws Throwable, we need to catch it here. Wrapping in Exception
feels weird but I'm not sure what's better to do.
...ns/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricsSender.java
Outdated
Show resolved
Hide resolved
...ns/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricsSender.java
Outdated
Show resolved
Hide resolved
...ns/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricsSender.java
Show resolved
Hide resolved
...ns/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricsSender.java
Outdated
Show resolved
Hide resolved
884d7c8
to
df32a06
Compare
...ns/micrometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricsSender.java
Outdated
Show resolved
Hide resolved
d7841a0
to
55da842
Compare
55da842
to
4394738
Compare
Introduces an immutable Request object and builder.
4394738
to
7fe776c
Compare
Signed-off-by: Johnny Lim <[email protected]>
Signed-off-by: Johnny Lim <[email protected]>
Introduces an immutable Request object and builder for convenience.
See review comments on #5994 and original PR #5691.