[processor/servicegraph] remove deprecated component #30149

codeboten · 2023-12-20T21:40:31Z

This component was replaced by the servicegraphconnector, which was a wrapper around the servicegraphprocessor. This PR moves the processor code into the connector component and removes the processor altogether. Note that as part of this PR I added the traces_to_traces support as it was supported by the processor before (with the development status).

Fixes #30041
Fixes #26091

mx-psi · 2023-12-21T09:32:58Z

panic: README.md does not exist at /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/processor/servicegraphprocessor/README.md, add one

songy23

Need to update feature gate registry too https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/7286377802/job/19855077048?pr=30149

codeboten · 2023-12-21T16:22:42Z

@songy23 i moved the flags from the processor to the connector.... not sure if i should have renamed them or not. maybe the code owners can weigh in on this @mapno @jpkrohling

songy23

Needs make generate-gh-issue-templates

atoulme · 2023-12-22T21:37:30Z

%v(0xe2e8a00,0xc00332d020)%v(0xe2e8a00,0xc003390720)--- FAIL: TestDefaultProcessors (0.02s)
    processors_test.go:150: 
        	Error Trace:	/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/cmd/otelcontribcol/processors_test.go:150
        	Error:      	Not equal: 
        	            	expected: 23
        	            	actual  : 24
        	Test:       	TestDefaultProcessors
        	Messages:   	All processors must be added to lifecycle tests
    --- FAIL: TestDefaultProcessors/servicegraph (0.00s)

mapno

Thanks for taking this on this 🙏 Left a couple of comments.

connector/servicegraphconnector/factory.go

mapno · 2024-01-02T11:51:59Z

connector/servicegraphconnector/connector.go

Making a file comment because I can't reference the lines I want.

IMO, naming should be migrated as well. So it'd be serviceGraphConnector, newConnector, etc.

The tracesConsumer can also be removed. It was used in the processor only.

I've updated the code to reflect this feedback. One more question what about the metrics generated by the component itself? Currently it looks like:

processor_dropped_spans processor_total_edges processor_expired_edges

I personally would change them as well. The processor is deprecated, and my assumption is that those metrics as well.

Having a connector emit metrics that have processor in name is confusing in the long run, I agree those metrics should be renamed. I'd be fine with renaming them in a separate pull request though.

A note on the renaming of the internal components - if I understand correctly, this is a refactoring (no change in behavior) and not needed to remove the Service Graph processor. I believe in future it's better to do it in a separate pull request. Makes the PRs smaller and easier to review.

@astencel-sumo fair enough. i was moving the code from the processor over and needed to rename a few things to make it all work.

github-actions · 2024-01-30T05:19:34Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

This component was replaced by the servicegraphconnector, which was a wrapper around the servicegraphprocessor. This PR moves the processor code into the connector component and removes the processor altogether. Note that as part of this PR I added the traces_to_traces support as it was supported by the processor before (with the development status). Fixes open-telemetry#30041 Fixes open-telemetry#26091 Signed-off-by: Alex Boten <aboten@lightstep.com>

Signed-off-by: Alex Boten <aboten@lightstep.com>

This follows the comment from open-telemetry#30149 (comment) Signed-off-by: Alex Boten <aboten@lightstep.com>

This follows the comment from #30149 (comment) --------- Signed-off-by: Alex Boten <aboten@lightstep.com>

This PR adds tests which were previously added in #17350, but got removed in #30149.

This PR adds tests which were previously added in open-telemetry#17350, but got removed in open-telemetry#30149.

codeboten requested a review from jpkrohling as a code owner December 20, 2023 21:40

codeboten requested a review from a team December 20, 2023 21:40

codeboten requested review from mx-psi and dmitryax as code owners December 20, 2023 21:40

github-actions bot assigned songy23 Dec 20, 2023

github-actions bot added cmd/configschema configschema command cmd/otelcontribcol otelcontribcol command connector/servicegraph processor/servicegraph Service graph processor labels Dec 20, 2023

mx-psi approved these changes Dec 21, 2023

View reviewed changes

songy23 reviewed Dec 21, 2023

View reviewed changes

codeboten requested a review from andrzej-stencel as a code owner December 21, 2023 18:26

github-actions bot added reports/distributions/contrib.yaml reports/distributions/sumo.yaml labels Dec 21, 2023

songy23 approved these changes Dec 21, 2023

View reviewed changes

bogdandrutu approved these changes Dec 21, 2023

View reviewed changes

JaredTan95 mentioned this pull request Dec 26, 2023

remove servicegraphprocessor open-telemetry/opentelemetry-collector-releases#445

Merged

mapno reviewed Jan 2, 2024

View reviewed changes

codeboten force-pushed the codeboten/mv-deprecated-component branch from 15b251a to 339fc5b Compare January 15, 2024 23:41

github-actions bot added the Stale label Jan 30, 2024

mapno approved these changes Jan 30, 2024

View reviewed changes

andrzej-stencel removed the Stale label Jan 30, 2024

andrzej-stencel approved these changes Jan 30, 2024

View reviewed changes

codeboten force-pushed the codeboten/mv-deprecated-component branch from 339fc5b to 183e212 Compare January 30, 2024 21:21

github-actions bot requested review from aboguszewski-sumo and kasia-kujawa January 30, 2024 21:22

github-actions bot requested review from pmcollins, rnishtala-sumo, sumo-drosiek and swiatekm January 30, 2024 21:22

codeboten force-pushed the codeboten/mv-deprecated-component branch from 8930949 to c56e183 Compare January 30, 2024 21:52

jpkrohling approved these changes Feb 1, 2024

View reviewed changes

codeboten force-pushed the codeboten/mv-deprecated-component branch from c9964e5 to d953a6b Compare February 1, 2024 19:24

Alex Boten added 8 commits February 1, 2024 15:31

rename & apply review feedback

0392d2b

Signed-off-by: Alex Boten <aboten@lightstep.com>

make generate

a6bfa48

Signed-off-by: Alex Boten <aboten@lightstep.com>

fix lint

eab5ba2

Signed-off-by: Alex Boten <aboten@lightstep.com>

remove createTraces

13ee31a

Signed-off-by: Alex Boten <aboten@lightstep.com>

clean up

7b336c1

Signed-off-by: Alex Boten <aboten@lightstep.com>

more cleanup

86248af

Signed-off-by: Alex Boten <aboten@lightstep.com>

ensure test calls shutdown

83ee263

Signed-off-by: Alex Boten <aboten@lightstep.com>

codeboten force-pushed the codeboten/mv-deprecated-component branch from d953a6b to 83ee263 Compare February 1, 2024 23:53

fix lint

f78af28

Signed-off-by: Alex Boten <aboten@lightstep.com>

codeboten merged commit d02d376 into open-telemetry:main Feb 2, 2024
142 checks passed

codeboten deleted the codeboten/mv-deprecated-component branch February 2, 2024 16:00

github-actions bot added this to the next release milestone Feb 2, 2024

codeboten mentioned this pull request Feb 2, 2024

[connector/servicegraph] rename metrics #31023

Merged

codeboten pushed a commit to codeboten/opentelemetry-collector-contrib that referenced this pull request Feb 2, 2024

[connector/servicegraph] rename metrics

c744880

This follows the comment from open-telemetry#30149 (comment) Signed-off-by: Alex Boten <aboten@lightstep.com>

codeboten pushed a commit that referenced this pull request Feb 5, 2024

[connector/servicegraph] rename metrics (#31023)

3952e23

This follows the comment from #30149 (comment) --------- Signed-off-by: Alex Boten <aboten@lightstep.com>

ptodev mentioned this pull request May 29, 2024

[connector/spanmetrics] Replace connector.servicegraph.virtualNode feature gate with config options #33282

Closed

ptodev mentioned this pull request Aug 20, 2024

[connector/spanmetrics] Add extra tests #34765

Merged

jpkrohling pushed a commit that referenced this pull request Aug 21, 2024

[connector/spanmetrics] Add extra tests (#34765)

4bac647

This PR adds tests which were previously added in #17350, but got removed in #30149.

f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this pull request Sep 12, 2024

[connector/spanmetrics] Add extra tests (open-telemetry#34765)

81ae34e

This PR adds tests which were previously added in open-telemetry#17350, but got removed in open-telemetry#30149.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[processor/servicegraph] remove deprecated component #30149

[processor/servicegraph] remove deprecated component #30149

codeboten commented Dec 20, 2023

mx-psi commented Dec 21, 2023

songy23 left a comment

codeboten commented Dec 21, 2023

songy23 left a comment

atoulme commented Dec 22, 2023

mapno left a comment

mapno Jan 2, 2024

mapno Jan 2, 2024

codeboten Jan 15, 2024

mapno Jan 30, 2024

andrzej-stencel Jan 30, 2024

andrzej-stencel Jan 30, 2024

codeboten Jan 30, 2024

github-actions bot commented Jan 30, 2024

[processor/servicegraph] remove deprecated component #30149

[processor/servicegraph] remove deprecated component #30149

Conversation

codeboten commented Dec 20, 2023

mx-psi commented Dec 21, 2023

songy23 left a comment

Choose a reason for hiding this comment

codeboten commented Dec 21, 2023

songy23 left a comment

Choose a reason for hiding this comment

atoulme commented Dec 22, 2023

mapno left a comment

Choose a reason for hiding this comment

mapno Jan 2, 2024

Choose a reason for hiding this comment

mapno Jan 2, 2024

Choose a reason for hiding this comment

codeboten Jan 15, 2024

Choose a reason for hiding this comment

mapno Jan 30, 2024

Choose a reason for hiding this comment

andrzej-stencel Jan 30, 2024

Choose a reason for hiding this comment

andrzej-stencel Jan 30, 2024

Choose a reason for hiding this comment

codeboten Jan 30, 2024

Choose a reason for hiding this comment

github-actions bot commented Jan 30, 2024