[extension/jaegerremotesampling] remove misleading agent endpoint #18070

frzifus · 2023-01-27T12:01:10Z

Description:

Removed / http endpoint from remotesampling extension.

Link to tracking Issue:

Closes [extension/jaegerremotesampling] Misleading implementation on two endpoints #18058

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

runforesight · 2023-01-27T12:02:26Z

Foresight Summary

Major Impacts

build-and-test-windows duration(4 seconds) has decreased 40 minutes 28 seconds compared to main branch avg(40 minutes 32 seconds).

View More Details

⭕ build-and-test-windows workflow has finished in 4 seconds (40 minutes 28 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

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

✅ check-links workflow has finished in 42 seconds (1 minute 8 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

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

✅ telemetrygen workflow has finished in 1 minute (1 minute 23 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

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

✅ tracegen workflow has finished in 1 minute 7 seconds (1 minute 18 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

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

✅ changelog workflow has finished in 1 minute 59 seconds and finished at 27th Jan, 2023.

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

✅ build-and-test workflow has finished in 35 minutes 33 seconds (16 minutes 52 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

Job	Failed Steps	Tests
unittest-matrix (1.19, internal)	- 🔗	✅ 561 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, internal)	- 🔗	✅ 561 ❌ 0 ⏭ 0 🔗	See Details
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, processor)	- 🔗	✅ 1471 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, extension)	- 🔗	✅ 546 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, processor)	- 🔗	✅ 1471 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 546 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-0)	- 🔗	✅ 2573 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	✅ 2573 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, exporter)	- 🔗	✅ 2414 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, exporter)	- 🔗	✅ 2414 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	✅ 1944 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-1)	- 🔗	✅ 1944 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, other)	- 🔗	✅ 4661 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, other)	- 🔗	✅ 4661 ❌ 0 ⏭ 0 🔗	See Details
integration-tests	- 🔗	✅ 55 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
check-collector-module-version	- 🔗	N/A	See Details
checks	- 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
check-codeowners	- 🔗	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 (extension)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	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-dev	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 3 minutes 27 seconds (4 minutes 1 second less than `main` branch avg.) and finished at 27th Jan, 2023.

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

✅ load-tests workflow has finished in 21 minutes 49 seconds (⚠️ 7 minutes 41 seconds more than `main` branch avg.) and finished at 27th Jan, 2023.

Job	Failed Steps	Tests
loadtest (TestIdleMode)	- 🔗	✅ 1 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceAttributesProcessor)	- 🔗	✅ 3 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetric10kDPS\|TestMetricsFromFile)	- 🔗	✅ 6 ❌ 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)	- 🔗	✅ 19 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details

🔎 See details on Foresight

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

codeboten

Is this not a breaking change?

jpkrohling · 2023-01-27T19:05:21Z

The current behavior is not in line with what Jaeger does, so, no current Jaeger clients work with this. But you are right, @codeboten, this is technically a breaking change.

Should we do this in two phases? First deprecate this, issuing a warn in the logs in case this endpoint gets called, removing it in N+2?

codeboten

so, no current Jaeger clients work with this

If that's the case I think it's probably ok to mark this as a breaking change (in case anyone out there is using this) and remove it in the same change.

.chloggen/remove_misleading_jaegerremotesampling_endpoint.yaml

yurishkuro · 2023-01-28T20:05:56Z

extension/jaegerremotesampling/internal/http.go

@@ -58,9 +58,6 @@ func NewHTTP(telemetry component.TelemetrySettings, settings confighttp.HTTPServ
 	}

 	srv.mux = http.NewServeMux()
-	// the legacy endpoint
-	srv.mux.Handle("/", http.HandlerFunc(srv.samplingStrategyHandler))
-
 	// the new endpoint -- not strictly necessary, as the previous one would match it


may want to fix the comment: "the previous one" doesn't exist anymore

Ups, good catch :X

[extension/jaegerremotesampling] remove misleading agent endpoint

8030236

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

frzifus requested a review from a team January 27, 2023 12:01

frzifus requested a review from jpkrohling as a code owner January 27, 2023 12:01

github-actions bot assigned MovieStoreGuy Jan 27, 2023

github-actions bot added the extension/jaegerremotesampling label Jan 27, 2023

jpkrohling approved these changes Jan 27, 2023

View reviewed changes

codeboten reviewed Jan 27, 2023

View reviewed changes

jpkrohling requested changes Jan 27, 2023

View reviewed changes

.chloggen/remove_misleading_jaegerremotesampling_endpoint.yaml Outdated Show resolved Hide resolved

Update .chloggen/remove_misleading_jaegerremotesampling_endpoint.yaml

ac1c4c7

codeboten approved these changes Jan 27, 2023

View reviewed changes

frzifus requested a review from jpkrohling January 28, 2023 07:39

MovieStoreGuy approved these changes Jan 28, 2023

View reviewed changes

jpkrohling approved these changes Jan 28, 2023

View reviewed changes

jpkrohling merged commit 650e624 into open-telemetry:main Jan 28, 2023

yurishkuro reviewed Jan 28, 2023

View reviewed changes

frzifus deleted the remove_misleading_jaegerremotesampling_endpoint branch January 28, 2023 20:18

This was referenced Feb 1, 2023

[extension/jaegerremotesampling] remove invalid comment #18230

Merged

Bump github.com/jaegertracing/jaeger from 1.41.0 to 1.42.0 causes issues #18401

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[extension/jaegerremotesampling] remove misleading agent endpoint #18070

[extension/jaegerremotesampling] remove misleading agent endpoint #18070

frzifus commented Jan 27, 2023

runforesight bot commented Jan 27, 2023 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 40 minutes 28 seconds compared to main branch avg(40 minutes 32 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (40 minutes 28 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ check-links workflow has finished in 42 seconds (1 minute 8 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ telemetrygen workflow has finished in 1 minute (1 minute 23 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ tracegen workflow has finished in 1 minute 7 seconds (1 minute 18 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ changelog workflow has finished in 1 minute 59 seconds and finished at 27th Jan, 2023.

✅ build-and-test workflow has finished in 35 minutes 33 seconds (16 minutes 52 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 3 minutes 27 seconds (4 minutes 1 second less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ load-tests workflow has finished in 21 minutes 49 seconds (⚠️ 7 minutes 41 seconds more than `main` branch avg.) and finished at 27th Jan, 2023.

codeboten left a comment

jpkrohling commented Jan 27, 2023

codeboten left a comment

yurishkuro Jan 28, 2023

frzifus Jan 28, 2023

[extension/jaegerremotesampling] remove misleading agent endpoint #18070

[extension/jaegerremotesampling] remove misleading agent endpoint #18070

Conversation

frzifus commented Jan 27, 2023

runforesight bot commented Jan 27, 2023 • edited Loading

Foresight Summary

build-and-test-windows duration(4 seconds) has decreased 40 minutes 28 seconds compared to main branch avg(40 minutes 32 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (40 minutes 28 seconds less than main branch avg.) and finished at 27th Jan, 2023.

✅ check-links workflow has finished in 42 seconds (1 minute 8 seconds less than main branch avg.) and finished at 27th Jan, 2023.

✅ telemetrygen workflow has finished in 1 minute (1 minute 23 seconds less than main branch avg.) and finished at 27th Jan, 2023.

✅ tracegen workflow has finished in 1 minute 7 seconds (1 minute 18 seconds less than main branch avg.) and finished at 27th Jan, 2023.

✅ changelog workflow has finished in 1 minute 59 seconds and finished at 27th Jan, 2023.

✅ build-and-test workflow has finished in 35 minutes 33 seconds (16 minutes 52 seconds less than main branch avg.) and finished at 27th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 3 minutes 27 seconds (4 minutes 1 second less than main branch avg.) and finished at 27th Jan, 2023.

✅ load-tests workflow has finished in 21 minutes 49 seconds (⚠️ 7 minutes 41 seconds more than main branch avg.) and finished at 27th Jan, 2023.

codeboten left a comment

Choose a reason for hiding this comment

jpkrohling commented Jan 27, 2023

codeboten left a comment

Choose a reason for hiding this comment

yurishkuro Jan 28, 2023

Choose a reason for hiding this comment

frzifus Jan 28, 2023

Choose a reason for hiding this comment

runforesight bot commented Jan 27, 2023 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 40 minutes 28 seconds compared to main branch avg(40 minutes 32 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (40 minutes 28 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ check-links workflow has finished in 42 seconds (1 minute 8 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ telemetrygen workflow has finished in 1 minute (1 minute 23 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ tracegen workflow has finished in 1 minute 7 seconds (1 minute 18 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ build-and-test workflow has finished in 35 minutes 33 seconds (16 minutes 52 seconds less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 3 minutes 27 seconds (4 minutes 1 second less than `main` branch avg.) and finished at 27th Jan, 2023.

✅ load-tests workflow has finished in 21 minutes 49 seconds (⚠️ 7 minutes 41 seconds more than `main` branch avg.) and finished at 27th Jan, 2023.