Add `_dd.base_service` tag by jbertran · Pull Request #3557 · DataDog/dd-trace-js

jbertran · 2023-08-21T14:06:15Z

What does this PR do?

Add _dd.base_service tag containing original tracer-wide configured service if it is not the service in produced spans.

Motivation

This is an initiative for every tracer that helps identify services that are modified in code from their DD_SERVICE default, to help remove nodes in service map that don't correspond to concrete services.

Plugin Checklist

Unit tests.

Additional Notes

github-actions · 2023-08-21T14:07:12Z

Overall package size

Self size: 5.18 MB
Deduped: 59.22 MB
No deduping: 59.39 MB

Dependency sizes

name	version	self size	total size
@datadog/native-iast-taint-tracking	1.5.0	14.86 MB	14.86 MB
@datadog/native-appsec	3.2.0	13.38 MB	13.39 MB
@datadog/pprof	3.2.0	10.8 MB	11.64 MB
protobufjs	7.2.4	2.74 MB	6.52 MB
@datadog/native-iast-rewriter	2.1.3	2.23 MB	2.32 MB
@opentelemetry/core	1.14.0	872.87 kB	1.47 MB
@datadog/native-metrics	2.0.0	898.77 kB	1.3 MB
@opentelemetry/api	1.4.1	780.32 kB	780.32 kB
import-in-the-middle	1.4.2	41.4 kB	704.79 kB
msgpack-lite	0.1.26	201.16 kB	281.59 kB
opentracing	0.14.7	194.81 kB	194.81 kB
semver	7.5.4	93.4 kB	123.8 kB
@datadog/sketches-js	2.1.0	109.9 kB	109.9 kB
lodash.sortby	4.7.0	75.76 kB	75.76 kB
lru-cache	7.14.0	74.95 kB	74.95 kB
ipaddr.js	2.1.0	60.23 kB	60.23 kB
ignore	5.2.4	51.22 kB	51.22 kB
int64-buffer	0.1.10	49.18 kB	49.18 kB
istanbul-lib-coverage	3.2.0	29.34 kB	29.34 kB
lodash.uniq	4.5.0	25.01 kB	25.01 kB
limiter	1.1.5	23.17 kB	23.17 kB
retry	0.13.1	18.85 kB	18.85 kB
lodash.kebabcase	4.1.1	17.75 kB	17.75 kB
node-abort-controller	3.1.1	16.89 kB	16.89 kB
lodash.pick	4.4.0	16.33 kB	16.33 kB
crypto-randomuuid	1.0.0	11.18 kB	11.18 kB
diagnostics_channel	1.1.0	7.07 kB	7.07 kB
path-to-regexp	0.1.7	6.78 kB	6.78 kB
koalas	1.0.2	6.47 kB	6.47 kB
methods	1.1.2	5.29 kB	5.29 kB
module-details-from-path	1.0.3	4.47 kB	4.47 kB

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

codecov · 2023-08-21T14:08:02Z

Codecov Report

Merging #3557 (e041be5) into master (1de0f95) will increase coverage by 0.04%.
Report is 3 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3557      +/-   ##
==========================================
+ Coverage   84.71%   84.76%   +0.04%     
==========================================
  Files         219      219              
  Lines        8949     8952       +3     
  Branches       33       33              
==========================================
+ Hits         7581     7588       +7     
+ Misses       1368     1364       -4

Files Changed	Coverage Δ
packages/dd-trace/src/format.js	`99.03% <100.00%> (+0.02%)`	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pr-commenter · 2023-08-22T09:54:10Z

Benchmarks

Benchmark execution time: 2023-09-13 15:52:04

Comparing candidate commit e041be5 in PR branch jbertran/dd-base-service with baseline commit 1de0f95 in branch master.

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

scenario:plugin-graphql-with-depth-and-collapse-off-18

🟥 max_rss_usage [+114.897KB; +171.695KB] or [+9.764%; +14.591%]

scenario:plugin-graphql-with-depth-and-collapse-on-18

🟥 max_rss_usage [+118.550KB; +155.042KB] or [+14.436%; +18.880%]

scenario:plugin-graphql-with-depth-off-18

🟥 max_rss_usage [+125.634KB; +142.818KB] or [+15.290%; +17.381%]

scenario:plugin-graphql-with-depth-on-max-18

🟥 max_rss_usage [+105.378KB; +164.534KB] or [+12.694%; +19.820%]

jbertran · 2023-09-08T14:36:32Z


    if (!Plugin) return
+
+    if (!this._tracerConfig) return // TODO: don't wait for tracer to be initialized


aws-sdk direct import tests (specifically, these) fail unless this action is inverted. I'm probably missing some understanding of the way tests set the tracer up, but in the current state, running these tests make the PluginManager instantiate plugins before it is given the tracer config, and causes plugins to be populated with a null tracer config in the PluginManager's registry.

Not relying on plugins solves this, but the comment below still holds IMO.

Looking at the existing flow, it doesn't make sense to me to allow plugins to be configured with a null tracer config before aborting if tracer config is null, and inverting these 2 steps fixes my test issue, but I may be missing something in tests instead.

tlhunter · 2023-09-13T20:33:45Z

The code seems fine but it looks like @iunanua had pointed out an issue

iunanua · 2023-09-14T07:13:08Z

The issue is already fixed 🎉
LGTM

* compute _dd.base_service in format * add _dd.base_service test for custom instrumentation * add generic plugin behaviour tests

jbertran force-pushed the jbertran/dd-base-service branch from e24ec5b to 5ba3baa Compare August 22, 2023 09:47

jbertran force-pushed the jbertran/dd-base-service branch from 5ba3baa to 2d05412 Compare August 22, 2023 09:54

jbertran added the semver-minor label Aug 22, 2023

jbertran marked this pull request as ready for review August 22, 2023 10:01

jbertran requested review from a team as code owners August 22, 2023 10:01

jbertran marked this pull request as draft August 23, 2023 13:46

jbertran force-pushed the jbertran/dd-base-service branch 2 times, most recently from 174a63e to ced3daa Compare August 25, 2023 12:28

jbertran force-pushed the jbertran/dd-base-service branch from c7b0f54 to 91174ba Compare September 8, 2023 09:00

jbertran marked this pull request as ready for review September 8, 2023 13:33

jbertran commented Sep 8, 2023

View reviewed changes

iunanua reviewed Sep 8, 2023

View reviewed changes

Comment thread packages/dd-trace/src/plugins/tracing.js Outdated

jbertran force-pushed the jbertran/dd-base-service branch 4 times, most recently from 265584f to 7029998 Compare September 13, 2023 13:24

Jordi Bertran de Balanda added 3 commits September 13, 2023 17:39

compute _dd.base_service in format

4328cb7

add _dd.base_service test for custom instrumentation

26b3ff8

add generic plugin behaviour tests

e041be5

jbertran force-pushed the jbertran/dd-base-service branch from 7029998 to e041be5 Compare September 13, 2023 15:44

iunanua approved these changes Sep 14, 2023

View reviewed changes

jbertran merged commit afc84ea into master Sep 14, 2023

khanayan123 mentioned this pull request Sep 26, 2023

V3.37.0 proposal #3655

Merged

khanayan123 mentioned this pull request Sep 26, 2023

V4.16.0 proposal #3657

Merged

khanayan123 pushed a commit that referenced this pull request Sep 26, 2023

Add _dd.base_service tag (#3557)

e1382c5

* compute _dd.base_service in format * add _dd.base_service test for custom instrumentation * add generic plugin behaviour tests

khanayan123 pushed a commit that referenced this pull request Sep 26, 2023

Add _dd.base_service tag (#3557)

87c9d71

* compute _dd.base_service in format * add _dd.base_service test for custom instrumentation * add generic plugin behaviour tests

khanayan123 pushed a commit that referenced this pull request Sep 27, 2023

Add _dd.base_service tag (#3557)

61fb2f2

* compute _dd.base_service in format * add _dd.base_service test for custom instrumentation * add generic plugin behaviour tests

khanayan123 pushed a commit that referenced this pull request Sep 27, 2023

Add _dd.base_service tag (#3557)

acf003f

* compute _dd.base_service in format * add _dd.base_service test for custom instrumentation * add generic plugin behaviour tests

tlhunter deleted the jbertran/dd-base-service branch September 28, 2023 16:22

erikvanbrakel mentioned this pull request Jul 5, 2024

[DEVX-3565] add neo4j plugin for v4.41 mazedesignhq/dd-trace-js#15

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `_dd.base_service` tag#3557

Add `_dd.base_service` tag#3557
jbertran merged 3 commits into
masterfrom
jbertran/dd-base-service

jbertran commented Aug 21, 2023 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 21, 2023 •

edited

Loading

Uh oh!

codecov Bot commented Aug 21, 2023 •

edited

Loading

Uh oh!

pr-commenter Bot commented Aug 22, 2023 •

edited

Loading

Uh oh!

jbertran Sep 8, 2023 •

edited

Loading

Uh oh!

Uh oh!

tlhunter commented Sep 13, 2023

Uh oh!

iunanua commented Sep 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		if (!Plugin) return

		if (!this._tracerConfig) return // TODO: don't wait for tracer to be initialized

Uh oh!

Conversation

jbertran commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Plugin Checklist

Additional Notes

Uh oh!

github-actions Bot commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Dependency sizes

Uh oh!

codecov Bot commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter Bot commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

scenario:plugin-graphql-with-depth-and-collapse-off-18

scenario:plugin-graphql-with-depth-and-collapse-on-18

scenario:plugin-graphql-with-depth-off-18

scenario:plugin-graphql-with-depth-on-max-18

Uh oh!

jbertran Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tlhunter commented Sep 13, 2023

Uh oh!

iunanua commented Sep 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jbertran commented Aug 21, 2023 •

edited

Loading

github-actions Bot commented Aug 21, 2023 •

edited

Loading

codecov Bot commented Aug 21, 2023 •

edited

Loading

pr-commenter Bot commented Aug 22, 2023 •

edited

Loading

jbertran Sep 8, 2023 •

edited

Loading