Prototype of removing bare span attribute keys#1097
Prototype of removing bare span attribute keys#1097jkwatson wants to merge 3 commits intoopen-telemetry:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1097 +/- ##
============================================
- Coverage 85.58% 85.55% -0.03%
- Complexity 1085 1087 +2
============================================
Files 138 138
Lines 3990 3968 -22
Branches 355 355
============================================
- Hits 3415 3395 -20
+ Misses 434 432 -2
Partials 141 141
Continue to review full report at Codecov.
|
|
Is there any relevant spec guidance, or guidance from other language's implementations that might be helpful in making this decision? |
|
Might be worth taking a look at the JavaScript codebase. Sounds like they have a lot of instrumentation, from what we heard this morning. |
|
That being said, for type-safe languages, we might have the most skin in the game at this point. |
bogdandrutu
left a comment
bogdandrutu
left a comment
There was a problem hiding this comment.
Overall it is not clear to me what is the benefit of having these new interfaces. Can you describe that in the description?
| package io.opentelemetry.common; | ||
|
|
||
| public interface BooleanValuedKey { | ||
| String key(); |
There was a problem hiding this comment.
What is the benefit of having this as an interface? Also have you thought about doing something like in the proto, extend the AttributeValue to be AttributeKeyValue?
There was a problem hiding this comment.
Yes! This was another possible suggestion in #1076 . There, we were calling it simply "Attribute". That's another possibility, but would introduce an extra allocation for each attribute application, which probably isn't desirable.
|
|
||
| package io.opentelemetry.trace.attributes; | ||
|
|
||
| import static io.opentelemetry.trace.attributes.LongAttributeSetter.longKey; |
There was a problem hiding this comment.
Attributes are used in other places like Resource would be good to have this helper in common.
|
I'm closing this in favor of work being prototyped in #1130 |
4 participants
As discussed extensively in #1076 , there is a desire to make the semantic attributes easier to apply in the context of a SpanBuilder. This suggestion replaces the bare
Stringkeys on the Span attribute setters, replacing them with interface-based methods that enforce the type of the value.This is a follow-on from the prototype in #1096, where the interfaces were introduced.
Prototype suggestion for #1076 , #1075