Port servicegraphprocessor to servicegraphconnector #18389

djaglowski · 2023-02-06T20:14:45Z

Similar to #18381. This is a copy/paste/update that results in a simpler codebase and config.

Still waiting for the framework to be merged into contrib before this can be fully validated, but it seemed like a pretty clean conversion.

runforesight · 2023-02-06T20:15:57Z

Foresight Summary

Major Impacts

TestLog10kDPS ❌ failed 1 times in 8 runs (12% fail rate).

TestLog10kDPS/FluentBitToOTLP ❌ failed 1 times in 8 runs (12% fail rate).

build-and-test-windows duration(3 seconds) has decreased 40 minutes 46 seconds compared to main branch avg(40 minutes 49 seconds).

View More Details

✅ Test Kubernetes Related Components workflow has finished in 16 minutes 59 seconds and finished at 6th Feb, 2023.

Job	Failed Steps	Tests
kubernetes-test	- 🔗	N/A	See Details

⭕ build-and-test-windows workflow has finished in 3 seconds (40 minutes 46 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
windows-unittest-matrix	- 🔗	N/A	See Details
windows-unittest	- 🔗	N/A	See Details

✅ telemetrygen workflow has finished in 1 minute (1 minute 55 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
build-dev	- 🔗	N/A	See Details
publish-latest	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ check-links workflow has finished in 3 minutes 12 seconds (⚠️ 44 seconds more than `main` branch avg.) and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
changed files	- 🔗	N/A	See Details
check-links	- 🔗	N/A	See Details

✅ changelog workflow has finished in 4 minutes 19 seconds and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
changelog	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 11 minutes 58 seconds (⚠️ 3 minutes 25 seconds more than `main` branch avg.) and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
prometheus-compliance-tests	- 🔗	✅ 21 ❌ 0 ⏭ 0 🔗	See Details

✅ e2e-tests workflow has finished in 15 minutes 22 seconds (3 minutes 20 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
kubernetes-test	- 🔗	N/A	See Details

❌ load-tests workflow has finished in 28 minutes 22 seconds (⚠️ 11 minutes 56 seconds more than `main` branch avg.) and finished at 10th Feb, 2023. 1 job failed. There are 2 test failures.

Job	Failed Steps	Tests
loadtest (TestIdleMode)	- 🔗	✅ 1 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetric10kDPS\|TestMetricsFromFile)	- 🔗	✅ 6 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceAttributesProcessor)	- 🔗	✅ 3 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceNoBackend10kSPS\|TestTrace1kSPSWithAttrs)	- 🔗	✅ 8 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceBallast1kSPSWithAttrs\|TestTraceBallast1kSPSAddAttrs)	- 🔗	✅ 10 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetricResourceProcessor\|TestTrace10kSPS)	- 🔗	✅ 12 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestBallastMemory\|TestLog10kDPS)	Loadtest 🔗	✅ 17 ❌ 2 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details

✅ build-and-test workflow has finished in 58 minutes 19 seconds and finished at 10th Feb, 2023.

Job	Failed Steps	Tests
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, internal)	- 🔗	✅ 561 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, internal)	- 🔗	✅ 561 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, extension)	- 🔗	✅ 537 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 537 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, processor)	- 🔗	✅ 1479 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, processor)	- 🔗	✅ 1479 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-0)	- 🔗	✅ 2574 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	✅ 2574 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, exporter)	- 🔗	✅ 2452 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, exporter)	- 🔗	✅ 2452 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-1)	- 🔗	✅ 1959 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	✅ 1959 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, other)	- 🔗	✅ 4675 ❌ 0 ⏭ 0 🔗	See Details
integration-tests	- 🔗	✅ 55 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, other)	- 🔗	✅ 4675 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
lint-matrix (receiver-0)	- 🔗	N/A	See Details
lint-matrix (receiver-1)	- 🔗	N/A	See Details
lint-matrix (processor)	- 🔗	N/A	See Details
lint-matrix (exporter)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
checks	- 🔗	N/A	See Details
check-collector-module-version	- 🔗	N/A	See Details
lint-matrix (extension)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	N/A	See Details
check-codeowners	- 🔗	N/A	See Details
lint	- 🔗	N/A	See Details
unittest (1.19)	- 🔗	N/A	See Details
unittest (1.18)	- 🔗	N/A	See Details
cross-compile (darwin, amd64)	- 🔗	N/A	See Details
cross-compile (darwin, arm64)	- 🔗	N/A	See Details
cross-compile (linux, 386)	- 🔗	N/A	See Details
cross-compile (linux, amd64)	- 🔗	N/A	See Details
cross-compile (linux, arm)	- 🔗	N/A	See Details
cross-compile (linux, arm64)	- 🔗	N/A	See Details
cross-compile (linux, ppc64le)	- 🔗	N/A	See Details
cross-compile (windows, 386)	- 🔗	N/A	See Details
cross-compile (windows, amd64)	- 🔗	N/A	See Details
build-package (deb)	- 🔗	N/A	See Details
build-package (rpm)	- 🔗	N/A	See Details
windows-msi	- 🔗	N/A	See Details
publish-check	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details
publish-dev	- 🔗	N/A	See Details

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

jpkrohling · 2023-02-07T12:20:19Z

cc @kovrus and @mapno

mapno

Thanks for the PR. TIL about connectors, and sound perfect for these types of components.

This PR will result in duplicated code bases, what are the following steps after this is merged? The processor gets marked as deprecated? I'd prefer to have both component at the same time for as short as possible.

jpkrohling · 2023-02-07T15:52:17Z

This PR will result in duplicated code bases, what are the following steps after this is merged? The processor gets marked as deprecated? I'd prefer to have both component at the same time for as short as possible.

What I have talked to @kovrus (and perhaps @djaglowski, not sure), is to have both versions available for some time, allowing for a generous grace period for users to migrate over. The config migration should be easy, but not effortless: they are different component types even though the component's config is identical. We have not identified reasons for metrics to not be backward compatible, so that part should be covered.

djaglowski · 2023-02-07T16:28:59Z

I agree with @jpkrohling's suggested process, with one caveat.

As excited as I am about the connectors framework, I think we should exercise caution in the short term. The ability to use connectors will be included in this week's release, but will be disabled by default, behind an alpha feature gate.

I think we can merge this whenever we are comfortable with the implementation but that we should not deprecate the processor and encourage migration until the connectors framework is enabled by default (beta stage of feature gate).

In other cases, I would suggest even more caution before recommending a migration. However, this processor is still in development, so requirements are minimal here.

mapno · 2023-02-08T10:57:55Z

What I'm worried about are both code bases diverging—eg. a contribution is being worked on in the processor (PR), that as soon as its merged will already make this PR outdated.

From what I've checked connector.Traces and consumer.Traces are interchangeable. Would it be possible to export newProcessor and call it in the factory in this pkg? I think that's a low-effort compromise to make maintenance a bit simpler. I don't mean to block or slow the migration.

djaglowski · 2023-02-08T14:06:34Z

@mapno, seems reasonable. I'll try reuse what can be reused in the processor.

djaglowski · 2023-02-08T17:00:01Z

I reworked this so that the entire implementation remains in the processor. This seems ideal because we do not need to export anything additional.

The code ends up with some if connector .. else ... but it seems not too bad. I think it will be easy enough to condense back down to the connectors-only implementation when the time comes.

.chloggen/servicegraphconnector.yaml

connector/servicegraphconnector/README.md

processor/servicegraphprocessor/factory_test.go

processor/servicegraphprocessor/config_test.go

kovrus

lgtm!

minor, some of the logs contain the servicegraphprocessor name which might be confusing for connector users.

processor/servicegraphprocessor/processor_test.go

djaglowski · 2023-02-10T18:33:38Z

I updated the log messages to account for the component type

djaglowski · 2023-02-10T19:20:22Z

I think this should be good to go, unless there is more feedback. @jpkrohling, do you want to take a look?

mapno reviewed Feb 7, 2023

View reviewed changes

djaglowski force-pushed the servicegraphconnector branch from 88735b5 to 00783ac Compare February 8, 2023 16:25

github-actions bot added the processor/servicegraph Service graph processor label Feb 8, 2023

github-actions bot requested a review from jpkrohling February 8, 2023 16:26

djaglowski force-pushed the servicegraphconnector branch 2 times, most recently from ccee8d1 to 60743fb Compare February 8, 2023 16:55

djaglowski added 2 commits February 8, 2023 19:11

Port servicegraphprocessor to servicegraphconnector

dd5f5d0

chlog

387b670

djaglowski force-pushed the servicegraphconnector branch from 3699079 to 387b670 Compare February 9, 2023 00:11

djaglowski marked this pull request as ready for review February 9, 2023 10:03

djaglowski requested a review from a team as a code owner February 9, 2023 10:03

github-actions bot assigned MovieStoreGuy Feb 9, 2023

kovrus reviewed Feb 9, 2023

View reviewed changes

Split out connector factory test

3f79c56

kovrus reviewed Feb 10, 2023

View reviewed changes

processor/servicegraphprocessor/processor_test.go Show resolved Hide resolved

djaglowski force-pushed the servicegraphconnector branch from 25e175a to e46b31e Compare February 10, 2023 16:28

kovrus approved these changes Feb 10, 2023

View reviewed changes

Remove extraneous note from readme

da122f6

djaglowski force-pushed the servicegraphconnector branch from e46b31e to da122f6 Compare February 10, 2023 21:38

djaglowski merged commit feb6c08 into open-telemetry:main Feb 11, 2023

djaglowski deleted the servicegraphconnector branch February 13, 2023 14:34

djaglowski mentioned this pull request Feb 13, 2023

Port spanmetrics processor to spanmetrics connector #18535

Merged

djaglowski mentioned this pull request Mar 15, 2023

[processor/servicegraph] Deprecate component #19737

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port servicegraphprocessor to servicegraphconnector #18389

Port servicegraphprocessor to servicegraphconnector #18389

djaglowski commented Feb 6, 2023

runforesight bot commented Feb 6, 2023 •

edited

Loading

`TestLog10kDPS` ❌ failed 1 times in 8 runs (12% fail rate).

`TestLog10kDPS/FluentBitToOTLP` ❌ failed 1 times in 8 runs (12% fail rate).

`build-and-test-windows` duration(3 seconds) has decreased 40 minutes 46 seconds compared to main branch avg(40 minutes 49 seconds).

✅ Test Kubernetes Related Components workflow has finished in 16 minutes 59 seconds and finished at 6th Feb, 2023.

⭕ build-and-test-windows workflow has finished in 3 seconds (40 minutes 46 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

✅ telemetrygen workflow has finished in 1 minute (1 minute 55 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

✅ check-links workflow has finished in 3 minutes 12 seconds (⚠️ 44 seconds more than `main` branch avg.) and finished at 10th Feb, 2023.

✅ changelog workflow has finished in 4 minutes 19 seconds and finished at 10th Feb, 2023.

✅ prometheus-compliance-tests workflow has finished in 11 minutes 58 seconds (⚠️ 3 minutes 25 seconds more than `main` branch avg.) and finished at 10th Feb, 2023.

✅ e2e-tests workflow has finished in 15 minutes 22 seconds (3 minutes 20 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

❌ load-tests workflow has finished in 28 minutes 22 seconds (⚠️ 11 minutes 56 seconds more than `main` branch avg.) and finished at 10th Feb, 2023. 1 job failed. There are 2 test failures.

✅ build-and-test workflow has finished in 58 minutes 19 seconds and finished at 10th Feb, 2023.

jpkrohling commented Feb 7, 2023

mapno left a comment

jpkrohling commented Feb 7, 2023

djaglowski commented Feb 7, 2023 •

edited

Loading

mapno commented Feb 8, 2023 •

edited

Loading

djaglowski commented Feb 8, 2023

djaglowski commented Feb 8, 2023

kovrus left a comment

djaglowski commented Feb 10, 2023

djaglowski commented Feb 10, 2023

Port servicegraphprocessor to servicegraphconnector #18389

Port servicegraphprocessor to servicegraphconnector #18389

Conversation

djaglowski commented Feb 6, 2023

runforesight bot commented Feb 6, 2023 • edited Loading

Foresight Summary

TestLog10kDPS ❌ failed 1 times in 8 runs (12% fail rate).

TestLog10kDPS/FluentBitToOTLP ❌ failed 1 times in 8 runs (12% fail rate).

build-and-test-windows duration(3 seconds) has decreased 40 minutes 46 seconds compared to main branch avg(40 minutes 49 seconds).

✅ Test Kubernetes Related Components workflow has finished in 16 minutes 59 seconds and finished at 6th Feb, 2023.

⭕ build-and-test-windows workflow has finished in 3 seconds (40 minutes 46 seconds less than main branch avg.) and finished at 10th Feb, 2023.

✅ telemetrygen workflow has finished in 1 minute (1 minute 55 seconds less than main branch avg.) and finished at 10th Feb, 2023.

✅ check-links workflow has finished in 3 minutes 12 seconds (⚠️ 44 seconds more than main branch avg.) and finished at 10th Feb, 2023.

✅ changelog workflow has finished in 4 minutes 19 seconds and finished at 10th Feb, 2023.

✅ prometheus-compliance-tests workflow has finished in 11 minutes 58 seconds (⚠️ 3 minutes 25 seconds more than main branch avg.) and finished at 10th Feb, 2023.

✅ e2e-tests workflow has finished in 15 minutes 22 seconds (3 minutes 20 seconds less than main branch avg.) and finished at 10th Feb, 2023.

❌ load-tests workflow has finished in 28 minutes 22 seconds (⚠️ 11 minutes 56 seconds more than main branch avg.) and finished at 10th Feb, 2023. 1 job failed. There are 2 test failures.

✅ build-and-test workflow has finished in 58 minutes 19 seconds and finished at 10th Feb, 2023.

jpkrohling commented Feb 7, 2023

mapno left a comment

Choose a reason for hiding this comment

jpkrohling commented Feb 7, 2023

djaglowski commented Feb 7, 2023 • edited Loading

mapno commented Feb 8, 2023 • edited Loading

djaglowski commented Feb 8, 2023

djaglowski commented Feb 8, 2023

kovrus left a comment

Choose a reason for hiding this comment

djaglowski commented Feb 10, 2023

djaglowski commented Feb 10, 2023

runforesight bot commented Feb 6, 2023 •

edited

Loading

`TestLog10kDPS` ❌ failed 1 times in 8 runs (12% fail rate).

`TestLog10kDPS/FluentBitToOTLP` ❌ failed 1 times in 8 runs (12% fail rate).

`build-and-test-windows` duration(3 seconds) has decreased 40 minutes 46 seconds compared to main branch avg(40 minutes 49 seconds).

⭕ build-and-test-windows workflow has finished in 3 seconds (40 minutes 46 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

✅ telemetrygen workflow has finished in 1 minute (1 minute 55 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

✅ check-links workflow has finished in 3 minutes 12 seconds (⚠️ 44 seconds more than `main` branch avg.) and finished at 10th Feb, 2023.

✅ prometheus-compliance-tests workflow has finished in 11 minutes 58 seconds (⚠️ 3 minutes 25 seconds more than `main` branch avg.) and finished at 10th Feb, 2023.

✅ e2e-tests workflow has finished in 15 minutes 22 seconds (3 minutes 20 seconds less than `main` branch avg.) and finished at 10th Feb, 2023.

❌ load-tests workflow has finished in 28 minutes 22 seconds (⚠️ 11 minutes 56 seconds more than `main` branch avg.) and finished at 10th Feb, 2023. 1 job failed. There are 2 test failures.

djaglowski commented Feb 7, 2023 •

edited

Loading

mapno commented Feb 8, 2023 •

edited

Loading