Add Exemplars to metrics generated in aggregate processor #3165
Conversation
Signed-off-by: Krishna Kondaka <[email protected]>
kkondaka
requested review from
chenqi0805,
engechas,
graytaylor0,
dinujoh,
cmanning09,
asifsmohammed,
dlvenable and
oeyh
as code owners
Signed-off-by: Krishna Kondaka <[email protected]>
| @@ -98,7 +98,7 @@ While not necessary, a great way to set up the Aggregate Processor [identificati | |||
| The values in a list are merely appended, so there can be duplicates. | |||
|
|
|||
| ### <a name="count"></a> | |||
| * `count`: Count Events belonging to the same group and generate a new event with values of the identification keys and the count, indicating the number of events. All Events that make up the combined Event will be dropped. | |||
| * `count`: Count Events belonging to the same group and generate a new event with values of the identification keys and the count, indicating the number of events. All Events that make up the combined Event will be dropped. One of the events is added as exemplar. If the aggregation is done on traces, then traceId and spanId are included in the exemplar, otherwise, spanId and traceId would be null. | |||
There was a problem hiding this comment.
GitHub appears to be reporting some sort of formatting error. Is the Markdown correct?
There was a problem hiding this comment.
I don't know. When you view it in "RichText Format", it seems to be fine to me.
| return new DefaultExemplar( | ||
| OTelProtoCodec.convertUnixNanosToISO8601(curTimeNanos), | ||
| 1.0, | ||
| event.get("spanId", String.class), // maybe null |
There was a problem hiding this comment.
This value should only be found when this is a SpanEvent. It may be better to check if this is a span and then handle as necessary.
e.g.
if(event instanceof SpanEvent) {
SpanEvent spanEvent = (SpanEvent) event;
String spanId = spanEvent.getSpanId();
String traceId = spanEvent.getTraceId();
}
return new DefaultExemplar(
OTelProtoCodec.convertUnixNanosToISO8601(curTimeNanos),
1.0,
spanId,
traceId,
attributes);
| @@ -56,13 +61,25 @@ private long getTimeNanos(Instant time) { | |||
| return currentTimeNanos; | |||
| } | |||
|
|
|||
| public Exemplar createExemplar(final Event event) { | |||
| long curTimeNanos = getTimeNanos(Instant.now()); | |||
| Map<String, Object> attributes = event.toMap(); | |||
There was a problem hiding this comment.
I'm not sure this is exactly what the attributes is meant to convey. In the model these are filtered_attributes.
And the current code in Data Prepper for metric exemplars uses only the provided filtered_attributes.
There was a problem hiding this comment.
Yeah. I do not know why SAP folks called it Attributes in the DataPrepper implementation
| long curTimeNanos = getTimeNanos(Instant.now()); | ||
| Map<String, Object> attributes = event.toMap(); | ||
| if (Objects.nonNull(id)) { | ||
| attributes.put("exemplar_id", id); |
There was a problem hiding this comment.
Is this the right naming convention? Does this work with the OpenSearch schemas?
There was a problem hiding this comment.
I changed it to exemplarId
Signed-off-by: Krishna Kondaka <[email protected]>
Description
Add Exemplars to metrics generated in aggregate processor
Resolves #3164
Issues Resolved
Resolves #3164
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.