Skip to content

appsec: support route quantization for proxies#3471

Merged
eliottness merged 6 commits into
mainfrom
eliottness/route-quantization
Apr 30, 2025
Merged

appsec: support route quantization for proxies#3471
eliottness merged 6 commits into
mainfrom
eliottness/route-quantization

Conversation

@eliottness

@eliottness eliottness commented Apr 28, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR create a small algorithm to support proxies where we don't have access to the route. This eliminate the need to support the case where there is no route provided. Which was already a noop since every framework provide it as of now. Except contribs working on proxies like envoyproxy/go-control-plane where the case where no route is provided re-emerged, and thus, the need to create a fake route with it.

Motivation

Better support for APIsec on proxies where the sampler was previously doing very wrong decisions when no route was provided

Testing

We extracted 2 millions spans from dogfooding orgs to test this using this code below. The data is in the drive here.

Depending on how we count we end up with 75% or 96% routes being right. Which is enough for a v0.

Details
func TestDogfoodingSpans(t *testing.T) {
	files, err := filepath.Glob("testdata/unique_spans_org_*.json")
	require.NoError(t, err)
	require.NotEmpty(t, files, "expected only one file to match the glob pattern")

	var total atomic.Uint32
	var wrong atomic.Uint32

	for _, file := range files {
		filename := filepath.Base(file)
		t.Run(filename, func(t *testing.T) {

			fp, err := os.Open(file)
			require.NoError(t, err)
			
			defer fp.Close()

			type span struct {
				Route      string `json:"route"`
				URL        string `json:"url"`
				StatusCode string `json:"status_code"`
				Language   string `json:"language"`
				Method     string `json:"method"`
				Service    string `json:"service"`
			}

			var parsed []span
			require.NoError(t, json.NewDecoder(fp).Decode(&parsed))

			for _, span := range parsed {
				if span.Route == "" {
					continue
				}

				if span.Route[0] != '/' {
					span.Route = "/" + span.Route
				}

				splitedRoute := strings.Split(span.Route, "/")
				wildcards := []rune{
					':',
					'{',
					'<',
					'(',
				}
				for i := 0; i < len(splitedRoute); i++ {
					for _, wildcard := range wildcards {
						if strings.Contains(splitedRoute[i], string(wildcard)) {
							splitedRoute[i] = "*"
						}
					}
				}

				// Reconstruct the route with the replaced segments
				span.Route = strings.TrimSuffix(strings.Join(splitedRoute, "/"), "/")

				var quantizer urlQuantizer
				result := quantizer.Quantize(span.Route)
				if result != span.Route {
					t.Logf("expected: %s, got: %s", span.Route, result)
					wrong.Add(1)
				}

				total.Add(1)
			}
		})
	}

	t.Logf("total: %d, wrong: %d", total.Load(), wrong.Load())
}

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running golangci-lint run locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the apm:ecosystem contrib/* related feature requests or bugs label Apr 28, 2025
@eliottness
eliottness requested a review from e-n-0 April 28, 2025 15:38
@eliottness eliottness added the appsec label Apr 28, 2025 — with Graphite App
@eliottness
eliottness marked this pull request as ready for review April 28, 2025 15:39
@eliottness
eliottness requested review from a team as code owners April 28, 2025 15:39
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Apr 28, 2025

Copy link
Copy Markdown

Datadog Report

Branch report: eliottness/route-quantization
Commit report: ff28599
Test service: dd-trace-go

✅ 0 Failed, 4517 Passed, 65 Skipped, 3m 51.24s Total Time

@pr-commenter

pr-commenter Bot commented Apr 28, 2025

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-04-29 15:48:49

Comparing candidate commit 7166df0 in PR branch eliottness/route-quantization with baseline commit c245968 in branch main.

Found 1 performance improvements and 5 performance regressions! Performance is the same for 49 metrics, 1 unstable metrics.

scenario:BenchmarkOTelApiWithCustomTags/datadog_otel_api-24

  • 🟩 execution_time [-176.090ns; -121.710ns] or [-3.413%; -2.359%]

scenario:BenchmarkSingleSpanRetention/no-rules-24

  • 🟥 execution_time [+5.553µs; +8.554µs] or [+2.115%; +3.258%]

scenario:BenchmarkSingleSpanRetention/with-rules/match-all-24

  • 🟥 execution_time [+5.679µs; +8.752µs] or [+2.152%; +3.317%]

scenario:BenchmarkSingleSpanRetention/with-rules/match-half-24

  • 🟥 execution_time [+5.418µs; +8.395µs] or [+2.056%; +3.185%]

scenario:BenchmarkStartSpan-24

  • 🟥 execution_time [+102.485ns; +125.115ns] or [+4.315%; +5.268%]

scenario:BenchmarkTracerAddSpans-24

  • 🟥 execution_time [+150.944ns; +184.456ns] or [+3.528%; +4.311%]

Comment thread instrumentation/appsec/emitter/httpsec/config.go Outdated
@eliottness
eliottness force-pushed the eliottness/route-quantization branch from be55239 to d302a23 Compare April 29, 2025 12:44
@eliottness
eliottness requested a review from a team as a code owner April 29, 2025 12:44
@eliottness
eliottness requested a review from e-n-0 April 29, 2025 12:51
Signed-off-by: Eliott Bouhana <[email protected]>

@genesor genesor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@eliottness
eliottness merged commit e237c3a into main Apr 30, 2025
@eliottness
eliottness deleted the eliottness/route-quantization branch April 30, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apm:ecosystem contrib/* related feature requests or bugs appsec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants