Skip to content

Commit 8b59df7

Browse files
ehsavoieehsavoie
committed
fix(opentelemetry): fix histogram serialization and error span attributes
- serializeMetrics: collect all histogram points into a JsonArray instead of overwriting a single key per iteration - OpenTelemetryErrorConsumer: drop misleading streaming-event attribute on error events; emit status code/description only - exportMetrics filter: use gen_ai. prefix allowlist instead of fragile name exclusions - Remove unused PUSH_NOTIFICATION_URL constant Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
1 parent 0126356 commit 8b59df7

6 files changed

Lines changed: 140 additions & 7 deletions

File tree

‎extras/opentelemetry/client/src/main/java/org/a2aproject/sdk/extras/opentelemetry/client/OpenTelemetryClientTransport.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -550,10 +550,9 @@ public void accept(Throwable t) {
550550
return;
551551
}
552552
AttributesBuilder builder = Attributes.builder();
553-
builder.put("gen_ai.agent.a2a.streaming-event", t.toString());
553+
builder.put("gen_ai.agent.a2a.status.code", StatusCode.ERROR.name());
554+
builder.put("gen_ai.agent.a2a.status.description", t.getMessage());
554555
try {
555-
builder.put("gen_ai.agent.a2a.status.code", StatusCode.ERROR.name());
556-
builder.put("gen_ai.agent.a2a.status.description", t.getMessage());
557556
delegate.accept(t);
558557
} finally {
559558
span.addEvent(name, builder.build());

‎extras/opentelemetry/client/src/test/java/org/a2aproject/sdk/extras/opentelemetry/client/OpenTelemetryClientTransportTest.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import org.a2aproject.sdk.spec.TaskIdParams;
2626
import org.a2aproject.sdk.spec.TaskPushNotificationConfig;
2727
import org.a2aproject.sdk.spec.TaskQueryParams;
28+
import io.opentelemetry.api.common.AttributeKey;
29+
import io.opentelemetry.api.common.Attributes;
2830
import io.opentelemetry.api.metrics.DoubleHistogram;
2931
import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
3032
import io.opentelemetry.api.metrics.Meter;
@@ -507,6 +509,31 @@ void sendMessage_noHistogramWhenNoMeter() throws A2AClientException {
507509
}
508510
}
509511

512+
@Test
513+
void testErrorConsumer_emitsStatusAttributesWithoutStreamingEventAttribute() throws A2AClientException {
514+
MessageSendParams request = mock(MessageSendParams.class);
515+
when(request.toString()).thenReturn("request-string");
516+
Consumer<Throwable> originalConsumer = mock(Consumer.class);
517+
518+
transport.sendMessageStreaming(request, mock(Consumer.class), originalConsumer, context);
519+
520+
ArgumentCaptor<Consumer<Throwable>> errorConsumerCaptor = ArgumentCaptor.forClass(Consumer.class);
521+
verify(delegate).sendMessageStreaming(eq(request), any(Consumer.class),
522+
errorConsumerCaptor.capture(), any(ClientCallContext.class));
523+
524+
RuntimeException error = new RuntimeException("stream broke");
525+
errorConsumerCaptor.getValue().accept(error);
526+
527+
ArgumentCaptor<Attributes> attrsCaptor = ArgumentCaptor.forClass(Attributes.class);
528+
verify(span).addEvent(eq(A2AMethods.SEND_STREAMING_MESSAGE_METHOD + "-error"), attrsCaptor.capture());
529+
530+
Attributes attrs = attrsCaptor.getValue();
531+
assertEquals(StatusCode.ERROR.name(), attrs.get(AttributeKey.stringKey("gen_ai.agent.a2a.status.code")));
532+
assertEquals("stream broke", attrs.get(AttributeKey.stringKey("gen_ai.agent.a2a.status.description")));
533+
assertNull(attrs.get(AttributeKey.stringKey("gen_ai.agent.a2a.streaming-event")),
534+
"streaming-event must not be set on an error span event");
535+
}
536+
510537
@Test
511538
void testSendMessageStreaming_ThrowsException() throws A2AClientException {
512539
MessageSendParams request = mock(MessageSendParams.class);

‎extras/opentelemetry/common/src/main/java/org/a2aproject/sdk/extras/opentelemetry/A2AObservabilityNames.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ public interface A2AObservabilityNames {
2323

2424
// Push notification
2525
String PUSH_NOTIFICATION_EVENT_KIND = GENAI_PREFIX + ".push_notification.event_kind";
26-
String PUSH_NOTIFICATION_URL = GENAI_PREFIX + ".push_notification.url";
2726

2827
// Metrics
2928
String GENAI_CLIENT_OPERATION_DURATION = "gen_ai.client.operation.duration";

‎extras/opentelemetry/integration-tests/pom.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,16 @@
8383
<artifactId>awaitility</artifactId>
8484
<scope>test</scope>
8585
</dependency>
86+
<dependency>
87+
<groupId>org.mockito</groupId>
88+
<artifactId>mockito-core</artifactId>
89+
<scope>test</scope>
90+
</dependency>
91+
<dependency>
92+
<groupId>org.mockito</groupId>
93+
<artifactId>mockito-junit-jupiter</artifactId>
94+
<scope>test</scope>
95+
</dependency>
8696
</dependencies>
8797

8898
<build>

‎extras/opentelemetry/integration-tests/src/main/java/org/a2aproject/sdk/extras/opentelemetry/it/A2ATestRoutes.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,15 +283,15 @@ public void exportMetrics(RoutingContext rc) {
283283
sdk.getSdkMeterProvider().forceFlush();
284284
}
285285
List<MetricData> metrics = inMemoryMetricExporter.getFinishedMetricItems().stream()
286-
.filter(m -> !m.getName().contains("export") && !m.getName().contains("reset"))
286+
.filter(m -> m.getName().startsWith("gen_ai."))
287287
.collect(Collectors.toList());
288288
rc.response()
289289
.setStatusCode(200)
290290
.putHeader(CONTENT_TYPE, APPLICATION_JSON)
291291
.end(gson.toJson(serializeMetrics(metrics)));
292292
}
293293

294-
private JsonElement serializeMetrics(List<MetricData> metricDataList) {
294+
JsonElement serializeMetrics(List<MetricData> metricDataList) {
295295
JsonArray result = new JsonArray(metricDataList.size());
296296
for (MetricData m : metricDataList) {
297297
JsonObject obj = new JsonObject();
@@ -302,13 +302,15 @@ private JsonElement serializeMetrics(List<MetricData> metricDataList) {
302302
if (m.getType() == MetricDataType.HISTOGRAM) {
303303
HistogramData histogram = m.getHistogramData();
304304
obj.addProperty("data_count", histogram.getPoints().size());
305+
JsonArray points = new JsonArray(histogram.getPoints().size());
305306
histogram.getPoints().forEach(point -> {
306307
JsonObject pointObj = new JsonObject();
307308
pointObj.addProperty("count", point.getCount());
308309
pointObj.addProperty("sum", point.getSum());
309310
point.getAttributes().forEach((k, v) -> pointObj.addProperty("attr_" + k.getKey(), v.toString()));
310-
obj.add("point", pointObj);
311+
points.add(pointObj);
311312
});
313+
obj.add("points", points);
312314
}
313315
result.add(obj);
314316
}

‎extras/opentelemetry/integration-tests/src/test/java/org/a2aproject/sdk/extras/opentelemetry/it/A2ATestRoutesTest.java‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package org.a2aproject.sdk.extras.opentelemetry.it;
2+
3+
import com.google.gson.JsonArray;
4+
import com.google.gson.JsonElement;
5+
import com.google.gson.JsonObject;
6+
import io.opentelemetry.api.common.Attributes;
7+
import io.opentelemetry.sdk.metrics.data.HistogramData;
8+
import io.opentelemetry.sdk.metrics.data.HistogramPointData;
9+
import io.opentelemetry.sdk.metrics.data.MetricData;
10+
import io.opentelemetry.sdk.metrics.data.MetricDataType;
11+
import org.junit.jupiter.api.Test;
12+
import org.junit.jupiter.api.extension.ExtendWith;
13+
import org.mockito.Mock;
14+
import org.mockito.junit.jupiter.MockitoExtension;
15+
16+
import java.util.List;
17+
18+
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
import static org.junit.jupiter.api.Assertions.assertFalse;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
import static org.mockito.Mockito.when;
22+
23+
@ExtendWith(MockitoExtension.class)
24+
class A2ATestRoutesTest {
25+
26+
@Mock
27+
private MetricData metricData;
28+
@Mock
29+
private HistogramData histogramData;
30+
@Mock
31+
private HistogramPointData point1;
32+
@Mock
33+
private HistogramPointData point2;
34+
35+
private final A2ATestRoutes routes = new A2ATestRoutes();
36+
37+
@Test
38+
void serializeMetrics_singleHistogramPoint_returnsPointsArray() {
39+
when(metricData.getName()).thenReturn("gen_ai.agent.a2a.streaming.duration");
40+
when(metricData.getDescription()).thenReturn("desc");
41+
when(metricData.getUnit()).thenReturn("s");
42+
when(metricData.getType()).thenReturn(MetricDataType.HISTOGRAM);
43+
when(metricData.getHistogramData()).thenReturn(histogramData);
44+
when(histogramData.getPoints()).thenReturn(List.of(point1));
45+
when(point1.getCount()).thenReturn(1L);
46+
when(point1.getSum()).thenReturn(0.5);
47+
when(point1.getAttributes()).thenReturn(Attributes.empty());
48+
49+
JsonElement result = routes.serializeMetrics(List.of(metricData));
50+
51+
JsonObject metric = result.getAsJsonArray().get(0).getAsJsonObject();
52+
assertTrue(metric.has("points"), "Should have 'points' JsonArray");
53+
assertFalse(metric.has("point"), "Should not have single-overwriting 'point' key");
54+
assertEquals(1, metric.getAsJsonArray("points").size());
55+
}
56+
57+
@Test
58+
void serializeMetrics_multipleHistogramPoints_allPointsSerialized() {
59+
when(metricData.getName()).thenReturn("gen_ai.agent.a2a.streaming.duration");
60+
when(metricData.getDescription()).thenReturn("desc");
61+
when(metricData.getUnit()).thenReturn("s");
62+
when(metricData.getType()).thenReturn(MetricDataType.HISTOGRAM);
63+
when(metricData.getHistogramData()).thenReturn(histogramData);
64+
when(histogramData.getPoints()).thenReturn(List.of(point1, point2));
65+
when(point1.getCount()).thenReturn(1L);
66+
when(point1.getSum()).thenReturn(0.5);
67+
when(point1.getAttributes()).thenReturn(Attributes.empty());
68+
when(point2.getCount()).thenReturn(3L);
69+
when(point2.getSum()).thenReturn(2.0);
70+
when(point2.getAttributes()).thenReturn(Attributes.empty());
71+
72+
JsonElement result = routes.serializeMetrics(List.of(metricData));
73+
74+
JsonObject metric = result.getAsJsonArray().get(0).getAsJsonObject();
75+
JsonArray points = metric.getAsJsonArray("points");
76+
assertEquals(2, points.size(), "Both histogram data points must be serialized");
77+
assertEquals(1L, points.get(0).getAsJsonObject().get("count").getAsLong());
78+
assertEquals(0.5, points.get(0).getAsJsonObject().get("sum").getAsDouble());
79+
assertEquals(3L, points.get(1).getAsJsonObject().get("count").getAsLong());
80+
assertEquals(2.0, points.get(1).getAsJsonObject().get("sum").getAsDouble());
81+
}
82+
83+
@Test
84+
void serializeMetrics_nonHistogramMetric_hasNoPointsKey() {
85+
when(metricData.getName()).thenReturn("gen_ai.client.operation.duration");
86+
when(metricData.getDescription()).thenReturn("desc");
87+
when(metricData.getUnit()).thenReturn("s");
88+
when(metricData.getType()).thenReturn(MetricDataType.LONG_SUM);
89+
90+
JsonElement result = routes.serializeMetrics(List.of(metricData));
91+
92+
JsonObject metric = result.getAsJsonArray().get(0).getAsJsonObject();
93+
assertFalse(metric.has("points"), "Non-histogram metric should not have a 'points' key");
94+
assertFalse(metric.has("point"), "Non-histogram metric should not have a 'point' key");
95+
}
96+
}

0 commit comments

Comments
 (0)