Simplified Declarative Configuration API, Option A#15671
Simplified Declarative Configuration API, Option A#15671trask merged 1 commit intoopen-telemetry:mainfrom
Conversation
2097a18 to
65baab2
Compare
65baab2 to
145cb50
Compare
| this.delegate = delegate; | ||
| } | ||
|
|
||
| public ExtendedDeclarativeConfigProperties get(String name) { |
There was a problem hiding this comment.
this is the new method in DeclarativeConfigProperties that is proposed in this PR
|
I like this variant over B - and in general
|
|
|
||
| private DeclarativeConfigUtil() {} | ||
|
|
||
| public static ExtendedDeclarativeConfigProperties get(OpenTelemetry openTelemetry) { |
There was a problem hiding this comment.
This method also makes the usage more concise - so it's really apart of the proposal I think
There was a problem hiding this comment.
possibly, I also don't mind the extra few charatcters of
GlobalOpenTelemetry.get().getConfigProvider().getInstrumentationConfig().
over
DeclarativeConfigUtil.get(GlobalOpenTelemetry.get()).
There was a problem hiding this comment.
I agree - but that requires the stabilization of config provider - which will take a while.
Option B could replace the existing default value methods
I wonder how useful this is? (and you can always get it via |
|
I like this over B as well. #15672 (comment) |
|
Cool, can I get an approval on this PR, and I'll move forward with this approach (I have several PRs pending this decision) I'll also continue to update anything retroactively based on @laurit's feedback. |
|
I slightly prefer option return over the current default value handling - but it's really only slightly. |
Exploring options for us to start using in this repository immediately, and to recommend upstream to the Declarative Configuration API.