Observation otel IT #297

brunobat · 2024-02-22T16:01:05Z

No description provided.

brunobat · 2024-12-04T18:46:43Z

Need to make the tests more resilient.

radcortez · 2024-12-16T16:11:02Z

...a/io/smallrye/opentelemetry/instrumentation/observation/cdi/ObservationRegistryProducer.java

+                        new OpenTelemetryObservationHandler(tracer)))
+                // todo duplicate the tracer strategy for baggage, adding a condition to bypass when no baggage is present
+                // todo just implement the receiver and sender handlers
+                // todo. Alternatively we can split in 2 the tracing handlers, one to create spans (in the current .observationHandler(new ObservationHandler.FirstMatchingCompositeObservationHandler )
+                // todo. A new .observationHandler bloc to process the baggage on the receiver side.
+                // todo. Another to propagate the context in a new .observationHandler )
+                // todo. We assume on the receiver side we open and close the baggage once because it should have just 1 scope app wide and the
+                // todo. user will use the baggage api itself. We are just making sure we don't break the propagation to the user.


Is this ok?

It means it doesn't provide baggage propagation through the Observation API. The OTel API can be used instead...
Implementing this is not trivial and the comment explains the possible approach.

radcortez · 2024-12-16T16:12:11Z

...emetry/instrumentation/observation/handler/PropagatingReceiverTracingObservationHandler.java

@@ -56,6 +56,7 @@ public String get(C carrier, String key) {
                return getter.get(carrier, key);
            }
        });
+        // extracted.makeCurrent(); // this actually fixes the baggage test


Is this ok?

Basically, if you uncomment, baggage propagation will be fixed, is there for reference... We can remove these comments but I will need to document these things somewhere, maybe on a README... What do you think?

radcortez · 2024-12-16T16:14:03Z

...rye/opentelemetry/instrumentation/observation/handler/AbstractTracingObservationHandler.java

+        final Attributes highCardAttributes = context.get(HIGH_CARD_ATTRIBUTES);
+        setOtelAttributes(span, highCardAttributes);
+
+        final Attributes lowCardAttributes = context.get(LOW_CARD_ATTRIBUTES);


Are these always set, no matter what?

The low cardinality ones, yes.
The high cardinality are only used in traces.
This is there to go around a limitation of the Observation API... Currently, It only supports storage of Strings. Will file a feature request there.

radcortez · 2024-12-16T16:16:17Z

...rye/opentelemetry/instrumentation/observation/handler/AbstractTracingObservationHandler.java

+
+        final Attributes lowCardAttributes = context.get(LOW_CARD_ATTRIBUTES);
+        setOtelAttributes(span, lowCardAttributes);
+
        for (KeyValue keyValue : context.getAllKeyValues()) {


Can it contain keys from HIGH_CARD_ATTRIBUTESand LOW_CARD_ATTRIBUTES?

yes, but originally stored in 2 different structures.

radcortez · 2024-12-16T16:19:50Z

...main/java/io/smallrye/opentelemetry/implementation/rest/observation/FilterDocumentation.java

+
+        @Override
+        public KeyName[] getHighCardinalityKeyNames() {
+            return KeyName.merge(HighCardinalityValues.values(), ServerHighCardinalityValues.values());


Probably cache the merge?

They are session scoped

radcortez · 2024-12-16T16:20:02Z

...main/java/io/smallrye/opentelemetry/implementation/rest/observation/FilterDocumentation.java

+
+        @Override
+        public KeyName[] getHighCardinalityKeyNames() {
+            return KeyName.merge(HighCardinalityValues.values(), ClientHighCardinalityValues.values());


Probably cache the merge?

They are session scoped

radcortez · 2024-12-16T16:20:41Z

...main/java/io/smallrye/opentelemetry/implementation/rest/observation/FilterDocumentation.java

+        }
+    };
+
+    public enum LowCardinalityValues implements KeyName {


Do the names come from some spec or it is our feeling?

From the OTel semconv, yes. I followed the latest versions

radcortez · 2024-12-16T16:21:24Z

.../java/io/smallrye/opentelemetry/implementation/rest/observation/ObservationClientFilter.java

+        builder.put(HTTP_ROUTE.getKey(), request.getUri().getPath().toString());// Fixme must contain a template /users/:userID?
+
+        builder.put(HTTP_REQUEST_METHOD, request.getMethod());// FIXME semantic conventions


Is this ok?

This needs updating. Will fix

brunobat force-pushed the observation-otel branch from e72ea9e to d10383f Compare February 22, 2024 16:41

brunobat changed the title ~~Observation otel~~ Observation otel IT Feb 25, 2024

radcortez force-pushed the main branch 4 times, most recently from df73490 to 58c8a10 Compare September 25, 2024 18:26

brunobat force-pushed the observation-otel branch 2 times, most recently from d81b882 to f4d5752 Compare November 28, 2024 18:05

brunobat added 6 commits December 4, 2024 18:39

Micrometer to Otel bridge

2e52ca7

Creation of a testing lib with the in memory OTel exporters

dcefe24

Observation OTel tracing bridge

f8ccc20

Add convention support

7d4b75f

Refactoring

e6e6b3c

New rest observation instrumentation and tests

664a2d9

brunobat force-pushed the observation-otel branch from f4d5752 to 7660fd5 Compare December 4, 2024 18:39

brunobat marked this pull request as ready for review December 4, 2024 18:40

brunobat requested a review from radcortez December 4, 2024 18:40

Final tweaks

198230b

brunobat force-pushed the observation-otel branch from 7660fd5 to 198230b Compare December 5, 2024 16:00

radcortez reviewed Dec 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Observation otel IT #297

Observation otel IT #297

brunobat commented Feb 22, 2024

brunobat commented Dec 4, 2024

radcortez Dec 16, 2024

brunobat Dec 16, 2024

radcortez Dec 16, 2024

brunobat Dec 16, 2024

radcortez Dec 16, 2024

brunobat Dec 16, 2024 •

edited

Loading

radcortez Dec 16, 2024

brunobat Dec 16, 2024 •

edited

Loading

radcortez Dec 16, 2024

brunobat Dec 16, 2024

radcortez Dec 16, 2024

brunobat Dec 16, 2024

radcortez Dec 16, 2024

brunobat Dec 16, 2024

radcortez Dec 16, 2024

brunobat Dec 16, 2024

		builder.put(HTTP_ROUTE.getKey(), request.getUri().getPath().toString());// Fixme must contain a template /users/:userID?

		builder.put(HTTP_REQUEST_METHOD, request.getMethod());// FIXME semantic conventions

Observation otel IT #297

Are you sure you want to change the base?

Observation otel IT #297

Conversation

brunobat commented Feb 22, 2024

brunobat commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunobat Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunobat Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunobat Dec 16, 2024 •

edited

Loading

brunobat Dec 16, 2024 •

edited

Loading