declarative config: support Span stacktrace#2262
Conversation
github-actions
Bot
requested review from
JonasKunz,
SylvainJuge and
jackshirazi
|
🔧 The result from spotlessApply was committed to the PR branch. |
| public class StackTraceComponentProvider implements ComponentProvider<SpanProcessor> { | ||
| @Override | ||
| public String getName() { | ||
| return "experimental-stacktrace"; |
There was a problem hiding this comment.
not sure if this has been discussed, but when using multi-word names, when should we use hyphens vs underscores? In cloudfoundry we use underscore, hyphen for xray lambda, underscore for Rule based routing sampler
Should we have some consistent convention?
There was a problem hiding this comment.
yes, should be underscores
|
@SylvainJuge please check again |
| public SpanProcessor create(DeclarativeConfigProperties config) { | ||
| return StackTraceAutoConfig.create( | ||
| new DeclarativeConfigPropertiesBridgeBuilder() | ||
| .addMapping(StackTraceAutoConfig.CONFIG_MIN_DURATION, "min_duration") |
There was a problem hiding this comment.
PR looks good to me and the comment below is not directly related to it.
When looking at this real life sample use of mappings I have an impression that it is easy to make a mistake in some cases. This is because order of adding mappings matters. If more strict mapping is added after more general, then it will never be executed, like
addMapping("aa.bb", "v1");
addMapping("aa.bb.cc", "v2");
What do you think about adding validation to DeclarativeConfigProperitesBridgeBuilder class to make sure that more specific mapping is always added earlier?
Instead of validation, we could sort the entries automatically in the builder by using TreeMap instead of LinkedHashMap.
There was a problem hiding this comment.
@robsunday can you open an issue for this so we don't lose it?
github-project-automation
Bot
moved this from Awaiting Review
to Done
in Declarative Configuration: Java
5 participants
Fixes #2028