Skip to content

fix: avoid using svc_src: m when service is among active integration defaults#17712

Open
emmettbutler wants to merge 138 commits intomainfrom
emmett.butler/sqlite3-snapshot-2
Open

fix: avoid using svc_src: m when service is among active integration defaults#17712
emmettbutler wants to merge 138 commits intomainfrom
emmett.butler/sqlite3-snapshot-2

Conversation

@emmettbutler
Copy link
Copy Markdown
Collaborator

@emmettbutler emmettbutler commented Apr 23, 2026

This change fixes an issue reported via Support in which svc_src is set to m in cases where service matches the _default_service of an active integration config. In such cases, the intended behavior is that it svc_src is equal to service.

The change also excludes the svc_src tag in cases where service == base_service.

@emmettbutler emmettbutler requested review from a team as code owners April 29, 2026 17:38
@emmettbutler emmettbutler requested a review from lym953 April 29, 2026 17:38
@pr-commenter
Copy link
Copy Markdown

pr-commenter Bot commented Apr 30, 2026

Benchmarks

Benchmark execution time: 2026-05-01 19:16:57

Comparing candidate commit 00c94e4 in PR branch emmett.butler/sqlite3-snapshot-2 with baseline commit e5ec646 in branch main.

Found 0 performance improvements and 3 performance regressions! Performance is the same for 584 metrics, 4 unstable metrics.

scenario:iastaspects-stringio_aspect

  • 🟥 execution_time [+543.156µs; +595.420µs] or [+14.042%; +15.394%]

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+83.315µs; +94.181µs] or [+19.416%; +21.948%]

scenario:telemetryaddmetric-1-count-metric-1-times

  • 🟥 execution_time [+181.133ns; +209.018ns] or [+8.597%; +9.921%]

Copy link
Copy Markdown
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

other than the perf feedback this lgtm

I did not review all snapshots, scanning the rough shape of changes makes sense to me though why they are changing here

Comment thread ddtrace/_trace/tracer.py Outdated
Copy link
Copy Markdown
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

question/suggestion about implementation, but otherwise lgtm

Comment thread ddtrace/internal/settings/_config.py Outdated
Copy link
Copy Markdown
Contributor

@rithikanarayan rithikanarayan left a comment

Choose a reason for hiding this comment

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

approved for serverless snapshots

Copy link
Copy Markdown
Contributor

@dubloom dubloom left a comment

Choose a reason for hiding this comment

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

My main question is:
Why some integrations remove _dd.svc_src if service == base_service and others are setting _dd.svc_src == service.

Also why are we deleting django snapshots ?

@emmettbutler
Copy link
Copy Markdown
Collaborator Author

emmettbutler commented May 4, 2026

@dubloom django snapshots are deleted because the tests pass without them. Presumably this is because they are associated with tests that have been deleted. The cases where svc_src == service are because of inferred base service.

@emmettbutler emmettbutler requested review from dubloom and wantsui May 4, 2026 16:22
Copy link
Copy Markdown
Collaborator

@wantsui wantsui left a comment

Choose a reason for hiding this comment

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

LGTM! I left some small comments about some of the snapshots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.