Improve pattern for validating and loading SDK extension plugins #7947
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7947 +/- ##
=========================================
Coverage 90.14% 90.15%
+ Complexity 7467 7457 -10
=========================================
Files 834 835 +1
Lines 22586 22520 -66
Branches 2240 2220 -20
=========================================
- Hits 20360 20302 -58
+ Misses 1527 1519 -8
Partials 699 699 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
...ubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfigUtil.java
Outdated
Show resolved
Hide resolved
| // We don't use the variable till later but call validate first to confirm there are not | ||
| // multiple samplers. | ||
| Map.Entry<String, DeclarativeConfigProperties> samplerKeyValue = | ||
| FileConfigUtil.validateSingleKeyValue(context, model, "composable sampler"); |
There was a problem hiding this comment.
It seems a little weird to just have a free-form string here as the "component name" parameter. Shouldn't this link directly to what field might be in the yaml, so it will be easier to debug when you do have a duplicate? Or at least something that will make it really obvious what might be duplicated so it's easy to find and correct?
There was a problem hiding this comment.
Yeah the error messaging throughout the declarative config implementation are lame right now in this respect. In the most recent java sig, @trask and I talked about extended DeclarativeConfigProperties to be to track the location of the node with respect to the document, and reference this location in error messages. This type of "document location context" is needed throughout for better error messages.
|
The approach seems reasonable; just a couple of comments to maybe tighten things up a bit. |
2 participants
This is a refactor aimed at improving and standardizing the pattern for loading SDK extension plugins. The key tasks we need to do:
opentelemetry-java- things like OTLP exporters, zipkin exporter, jaeger sampler. Other types are custom and provided by external packages. We want to use the convenience of the well known model types when available. And load via SPI when we don't. In all cases, we want the pattern to be clean and consistent.