Add otel-apm-service-map processor#6479
Conversation
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/OtelApmServiceMapProcessor.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opensearch/dataprepper/plugins/processor/utils/ApmServiceMapMetricsUtil.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opensearch/dataprepper/plugins/processor/utils/ApmServiceMapMetricsUtil.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opensearch/dataprepper/plugins/processor/utils/ApmServiceMapMetricsUtil.java
Outdated
Show resolved
Hide resolved
...earch/dataprepper/plugins/processor/otel_apm_service_map/utils/ApmServiceMapMetricsUtil.java
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/OtelApmServiceMapProcessor.java
Outdated
Show resolved
Hide resolved
...c/test/java/org/opensearch/dataprepper/plugins/processor/OtelApmServiceMapProcessorTest.java
Outdated
Show resolved
Hide resolved
KarstenSchnitter
left a comment
There was a problem hiding this comment.
I have just started reviewing this PR. I am not quite getting the entire goal of the change, yet. But I found an issue with the maps used in the OtelApmServiceMapProcessor. You should not use byte arrays as map keys, as the comparisons do not work properly (they are compared using references not the array values). See, e.g., https://www.baeldung.com/java-map-key-byte-array for an explanation.
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...r/src/main/java/org/opensearch/dataprepper/plugins/processor/OtelApmServiceMapProcessor.java
Outdated
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
|
|
||
| final Map<MetricKey, MetricAggregationState> metricsStateByKey = new HashMap<>(); | ||
|
|
||
| final Map<byte[], Collection<SpanStateData>> previousSpansByTraceId = buildSpansByTraceIdMap(previousWindow); |
There was a problem hiding this comment.
This map should already have been calculated. Is there a way to reuse the last result where previousWindow was currentWindow?
There was a problem hiding this comment.
Not sure I understand your point. The windows maintained so that we handle the cases where map could not be built because of some slow (out of order) data.
fda46b6 to
f64ff93
Compare
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
Co-authored-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>, Neeraj Kumar <kneeraj@amazon.com> Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
f64ff93 to
5db9901
Compare
dlvenable
left a comment
There was a problem hiding this comment.
I resolved the conversations on the completed items. I still have a few comments. And @KarstenSchnitter also has some good points we should address.
...earch/dataprepper/plugins/processor/otel_apm_service_map/utils/ApmServiceMapMetricsUtil.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
dlvenable
left a comment
There was a problem hiding this comment.
Thanks @kkondaka
@KarstenSchnitter , Can you take another look and approve if it is good for you? We'd like to release this as experimental in 2.14.
|
@KarstenSchnitter Please review again. I cannot merge while you have "request for changes" |
KarstenSchnitter
left a comment
There was a problem hiding this comment.
LGTM, can you answer my question on the peer forwarding? Out of curiosity I would like to know.
...pensearch/dataprepper/plugins/processor/otel_apm_service_map/OTelApmServiceMapProcessor.java
Show resolved
Hide resolved
...lugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ClasspathPluginProvider.java
Outdated
Show resolved
Hide resolved
...mmon/src/main/java/org/opensearch/dataprepper/plugins/otel/utils/OTelSpanDerivationUtil.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@amazon.com>
dlvenable
left a comment
There was a problem hiding this comment.
This looks good @kkondaka . Let's be sure the tests pass (maybe not the Kafka ones).
@KarstenSchnitter can you re-review?
Description
The
otel_apm_service_mapprocessor analyzes OpenTelemetry trace spans to automatically generate Application Performance Monitoring (APM) service map relationships and metrics. It creates structured events that can be visualized as service topology graphs, showing how services communicate with each other and their performance characteristics.Issues Resolved
Resolves #6482
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.