Remove support for OTEL spanmetrics processor #5539

varshith257 · 2024-06-05T04:52:16Z

Which problem is this PR solving?

Part of [deprecate] Remove support for OTEL spanmetrics processor #4736

Description of the changes

removed support for the spanmetrics processor flag.

How was this change tested?

CI

Checklist

I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
I have signed all commits
I have added unit tests for the new functionality
I have run lint and test steps successfully
- for jaeger: make lint test
- for jaeger-ui: yarn lint and yarn test

Signed-off-by: Vamshi Maskuri <[email protected]>

…eger into cleanup-processor

codecov · 2024-06-05T05:06:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.20%. Comparing base (6f2fe36) to head (f5ff0d7).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5539      +/-   ##
==========================================
- Coverage   96.20%   96.20%   -0.01%     
==========================================
  Files         327      327              
  Lines       16021    16006      -15     
==========================================
- Hits        15413    15398      -15     
  Misses        432      432              
  Partials      176      176

Flag	Coverage Δ
badger_v1	`8.05% <ø> (ø)`
badger_v2	`1.93% <ø> (ø)`
cassandra-3.x-v1	`16.45% <ø> (ø)`
cassandra-3.x-v2	`1.85% <ø> (ø)`
cassandra-4.x-v1	`16.45% <ø> (ø)`
cassandra-4.x-v2	`1.85% <ø> (ø)`
elasticsearch-7.x-v1	`18.88% <ø> (ø)`
elasticsearch-8.x-v1	`19.09% <ø> (ø)`
elasticsearch-8.x-v2	`19.07% <ø> (ø)`
grpc_v1	`9.48% <ø> (+0.01%)`	⬆️
grpc_v2	`7.53% <ø> (ø)`
kafka	`9.77% <ø> (ø)`
opensearch-1.x-v1	`18.94% <ø> (ø)`
opensearch-2.x-v1	`18.94% <ø> (ø)`
opensearch-2.x-v2	`18.94% <ø> (+0.01%)`	⬆️
unittests	`94.07% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

varshith257 · 2024-06-05T05:15:40Z

@yurishkuro Still there any to remove? These are files mainly updated in the PR linked in the ticket for dual support of the processor and connector.

And for README, can we keep the migration guide or remove it?

docker-compose/monitor/README.md

plugin/metrics/prometheus/options.go

docker-compose/monitor/README.md

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 · 2024-06-05T09:31:38Z

@albertteoh PTAL. I have also removed the migration guide. Would you also like me to update the configuration of the spanmetrics connector replacing the migration guide there?

Signed-off-by: Vamshi Maskuri <[email protected]>

plugin/metrics/prometheus/options.go

Signed-off-by: Vamshi Maskuri <[email protected]>

plugin/metrics/prometheus/metricsstore/reader.go

Signed-off-by: Vamshi Maskuri <[email protected]>

albertteoh · 2024-06-05T11:59:41Z

Looks like the tests need updating.

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 · 2024-06-05T12:08:00Z

Yeah! Just fixed now

varshith257 · 2024-06-05T12:38:24Z

@albertteoh The tests are failing as I update with the operation label as expected and converse it failing again with no need for an operation label :)

Before:
https://github.com/jaegertracing/jaeger/actions/runs/9381948382/job/25832322664

            	Error Trace:	/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:956
            	            				/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:483
            	Error:      	Not equal: 
            	            	expected: 2
            	            	actual  : 1
            	Test:       	TestGetErrorRates/enable_support_for_spanmetrics_connector_with_normalized_metric_name
        reader_test.go:962: 
            	Error Trace:	/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:962
            	            				/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:483
            	Error:      	Should be empty, but was map[operation:/OrderResult]
            	Test:       	TestGetErrorRates/enable_support_for_spanmetrics_connector_with_normalized_metric_name
FAIL

and this one after updating tests as fails in above unit tests fail:

https://github.com/jaegertracing/jaeger/actions/runs/9383740294/job/25838045548
            	Test:       	TestGetErrorRates/enable_support_for_spanmetrics_connector_with_normalized_metric_name
        reader_test.go:950: 
            	Error Trace:	/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:950
            	            				/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:477
            	Error:      	Not equal: 
            	            	expected: 1
            	            	actual  : 2
            	Test:       	TestGetErrorRates/enable_support_for_spanmetrics_connector_with_normalized_metric_name
        reader_test.go:952: 
            	Error Trace:	/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:952
            	            				/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:477
            	Error:      	map[string]string{"service_name":"emailservice"} does not contain "operation"
            	Test:       	TestGetErrorRates/enable_support_for_spanmetrics_connector_with_normalized_metric_name
        reader_test.go:953: 
            	Error Trace:	/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:953
            	            				/home/runner/work/jaeger/jaeger/plugin/metrics/prometheus/metricsstore/reader_test.go:477
            	Error:      	Not equal: 
            	            	expected: ""
            	            	actual  : "/OrderResult"

The current definitions of the functions GetLatencies, GetCallRates, and GetErrorRates in the reader.go, it seems these functions are designed to return ErrDisabled indicating that the metrics fetching functionality is disabled or not implemented

Any suggestions on it @albertteoh @yurishkuro ?

plugin/metrics/prometheus/metricsstore/reader_test.go

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 · 2024-06-06T12:06:57Z

@albertteoh It's 100% hit of test coverage now for this patch :)

varshith257 · 2024-06-07T05:26:57Z

@yurishkuro PTAL and review the changes

plugin/metrics/prometheus/metricsstore/reader_test.go

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 · 2024-06-07T14:57:29Z

@albertteoh @yurishkuro have added back the var operation => span_name. It fails tests and creates confusion about the already existing span_name variable there. Isn't it span.name instead span_name as mentioned in the migration guide

The operation metric attribute was renamed to span.name.

Signed-off-by: Vamshi Maskuri <[email protected]>

albertteoh · 2024-06-08T12:02:43Z

Isn't it span.name instead span_name as mentioned in the migration guide

@varshith257 From: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheus/README.md#labels

You'll see that one of the normalization steps performed to convert an OTLP attribute name to Prometheus is to: "Drop unsupported characters and replace with underscores (_)"

The example given there is to replace host.name -> host_name.

varshith257 · 2024-06-08T13:12:38Z

The two lines mentioned for changing in fn of mock Prometheus Server have updated to span_name. After debugging the metricLabel in reader.go is still pointing to operation in tests though we already changed in reader go. But, I can't pinpoint where still this operation label is emitting from, though we have removed most of it references

Error: map[string]string{"service_name":"emailservice"} does not contain "operation"

I have updated the mapping function for it in assert labels to map operation => span_name

…leanup-processor

Signed-off-by: Vamshi Maskuri <[email protected]>

plugin/metrics/prometheus/metricsstore/reader_test.go

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 · 2024-06-11T03:00:54Z

@albertteoh PTAL

plugin/metrics/prometheus/metricsstore/reader_test.go

## Which problem is this PR solving? - Resolves #5672 - Due to oversight, a bug was introduced in #5539 which changed the attribute name expected by the UI from `operation` to `span_name` ## Description of the changes - Revert those changes ## How was this change tested? `(cd docker-compose/monitor/ && make build dev)` <img width="1910" alt="image" src="https://github.com/jaegertracing/jaeger/assets/3523016/79400073-75f1-4db5-8bef-9a4c439d2852"> ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: Yuri Shkuro <[email protected]>

yurishkuro · 2024-06-22T19:59:08Z

reverted changes to "operation" label #5673

## Which problem is this PR solving? - Resolves #5672 - Due to oversight, a bug was introduced in #5539 which changed the attribute name expected by the UI from `operation` to `span_name` ## Description of the changes - Revert those changes ## How was this change tested? `(cd docker-compose/monitor/ && make build dev)` <img width="1910" alt="image" src="https://github.com/jaegertracing/jaeger/assets/3523016/79400073-75f1-4db5-8bef-9a4c439d2852"> ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: Yuri Shkuro <[email protected]>

cleanup processor

a2aedf6

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 requested a review from a team as a code owner June 5, 2024 04:52

varshith257 requested a review from joe-elliott June 5, 2024 04:52

varshith257 and others added 3 commits June 5, 2024 10:22

Merge branch 'main' into cleanup-processor

807fbbd

fix lint

d365755

Signed-off-by: Vamshi Maskuri <[email protected]>

Merge branch 'cleanup-processor' of https://github.com/varshith257/ja…

13e21bc

…eger into cleanup-processor

albertteoh reviewed Jun 5, 2024

View reviewed changes

docker-compose/monitor/README.md Outdated Show resolved Hide resolved

plugin/metrics/prometheus/options.go Outdated Show resolved Hide resolved

plugin/metrics/prometheus/options.go Outdated Show resolved Hide resolved

docker-compose/monitor/README.md Outdated Show resolved Hide resolved

varshith257 added 2 commits June 5, 2024 14:52

remove flags

4673048

Signed-off-by: Vamshi Maskuri <[email protected]>

remove migration guide

726c572

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 requested a review from albertteoh June 5, 2024 09:31

fix lint

ea54368

Signed-off-by: Vamshi Maskuri <[email protected]>

albertteoh reviewed Jun 5, 2024

View reviewed changes

plugin/metrics/prometheus/options.go Show resolved Hide resolved

varshith257 added 3 commits June 5, 2024 15:12

cleanup config

be843b7

Signed-off-by: Vamshi Maskuri <[email protected]>

more cleanup

c669b38

Signed-off-by: Vamshi Maskuri <[email protected]>

fix

1a9cf17

Signed-off-by: Vamshi Maskuri <[email protected]>

albertteoh reviewed Jun 5, 2024

View reviewed changes

plugin/metrics/prometheus/metricsstore/reader.go Outdated Show resolved Hide resolved

plugin/metrics/prometheus/metricsstore/reader.go Outdated Show resolved Hide resolved

varshith257 requested a review from albertteoh June 5, 2024 09:57

varshith257 added 2 commits June 5, 2024 16:32

fix unit tests

82cbc37

Signed-off-by: Vamshi Maskuri <[email protected]>

refactor connector vars

9c55ce0

Signed-off-by: Vamshi Maskuri <[email protected]>

refactor unit tests

92d2db2

Signed-off-by: Vamshi Maskuri <[email protected]>

albertteoh reviewed Jun 6, 2024

View reviewed changes

plugin/metrics/prometheus/metricsstore/reader_test.go Outdated Show resolved Hide resolved

varshith257 and others added 3 commits June 6, 2024 16:47

fix tests

53f1870

Signed-off-by: Vamshi Maskuri <[email protected]>

fix more tests

11a6703

Signed-off-by: Vamshi Maskuri <[email protected]>

Merge branch 'main' into cleanup-processor

4924c75

varshith257 requested a review from albertteoh June 6, 2024 12:14

yurishkuro reviewed Jun 7, 2024

View reviewed changes

plugin/metrics/prometheus/metricsstore/reader_test.go Show resolved Hide resolved

plugin/metrics/prometheus/metricsstore/reader_test.go Show resolved Hide resolved

yurishkuro added the changelog:breaking-change Change that is breaking public APIs or established behavior label Jun 7, 2024

Merge branch 'main' into cleanup-processor

1753099

varshith257 requested a review from yurishkuro June 7, 2024 07:51

refactor tests

7e0e1c9

Signed-off-by: Vamshi Maskuri <[email protected]>

fix

d70a0d9

Signed-off-by: Vamshi Maskuri <[email protected]>

varshith257 added 3 commits June 8, 2024 18:54

Merge branch 'main' of https://github.com/jaegertracing/jaeger into c…

24fd1e7

…leanup-processor

fix lint

9fbfeb9

Signed-off-by: Vamshi Maskuri <[email protected]>

refactor tests

e1450d2

Signed-off-by: Vamshi Maskuri <[email protected]>

yurishkuro reviewed Jun 9, 2024

View reviewed changes

plugin/metrics/prometheus/metricsstore/reader_test.go Show resolved Hide resolved

varshith257 and others added 2 commits June 10, 2024 08:19

enhanced logging and fix tests

1bbc72f

Signed-off-by: Vamshi Maskuri <[email protected]>

Update to_domain.go

7d04bbf

Signed-off-by: Vamshi Maskuri <[email protected]>

Merge branch 'main' into cleanup-processor

f5ff0d7

albertteoh approved these changes Jun 11, 2024

View reviewed changes

plugin/metrics/prometheus/metricsstore/reader_test.go Show resolved Hide resolved

yurishkuro merged commit ada5569 into jaegertracing:main Jun 11, 2024
40 checks passed

varshith257 deleted the cleanup-processor branch June 11, 2024 14:12

This was referenced Jun 11, 2024

Prepare release v1.58.0 #5557

Merged

[deprecate] Remove support for OTEL spanmetrics processor #4736

Closed

blueswen mentioned this pull request Jun 22, 2024

[Bug]: Operation metrics do not show up in SPM #5672

Closed

yurishkuro mentioned this pull request Jun 22, 2024

Restore "operation" name in the metrics response #5673

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove support for OTEL spanmetrics processor #5539

Remove support for OTEL spanmetrics processor #5539

varshith257 commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 5, 2024 •

edited

Loading

varshith257 commented Jun 5, 2024

varshith257 commented Jun 5, 2024

albertteoh commented Jun 5, 2024

varshith257 commented Jun 5, 2024 •

edited

Loading

varshith257 commented Jun 5, 2024 •

edited

Loading

varshith257 commented Jun 6, 2024 •

edited

Loading

varshith257 commented Jun 7, 2024

varshith257 commented Jun 7, 2024 •

edited

Loading

albertteoh commented Jun 8, 2024

varshith257 commented Jun 8, 2024 •

edited

Loading

varshith257 commented Jun 11, 2024

yurishkuro commented Jun 22, 2024

Remove support for OTEL spanmetrics processor #5539

Remove support for OTEL spanmetrics processor #5539

Conversation

varshith257 commented Jun 5, 2024 • edited Loading

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

codecov bot commented Jun 5, 2024 • edited Loading

Codecov Report

varshith257 commented Jun 5, 2024

varshith257 commented Jun 5, 2024

albertteoh commented Jun 5, 2024

varshith257 commented Jun 5, 2024 • edited Loading

varshith257 commented Jun 5, 2024 • edited Loading

varshith257 commented Jun 6, 2024 • edited Loading

varshith257 commented Jun 7, 2024

varshith257 commented Jun 7, 2024 • edited Loading

albertteoh commented Jun 8, 2024

varshith257 commented Jun 8, 2024 • edited Loading

varshith257 commented Jun 11, 2024

yurishkuro commented Jun 22, 2024

varshith257 commented Jun 5, 2024 •

edited

Loading

codecov bot commented Jun 5, 2024 •

edited

Loading

varshith257 commented Jun 5, 2024 •

edited

Loading

varshith257 commented Jun 5, 2024 •

edited

Loading

varshith257 commented Jun 6, 2024 •

edited

Loading

varshith257 commented Jun 7, 2024 •

edited

Loading

varshith257 commented Jun 8, 2024 •

edited

Loading