Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace manual.keep tag usage with an specific method to keep the trace #4739

Merged
merged 7 commits into from
Oct 29, 2024

Conversation

iunanua
Copy link
Contributor

@iunanua iunanua commented Sep 30, 2024

What does this PR do?

  • Incorporate the new PrioritySampler.setPriority(span, samplingPriority, mechanism) method to allow to set an span priority directly.
  • Replace the usage of manual.keep tag in appsec and iast modules with the PrioritySampler.keepTrace() method.

Motivation

Modules like appsec and iast use manual.keep tag when they want to indicate the tracer to keep the trace.
But this is not entirely correct because some times priority has been set on the creation of the trace and adding manual.keep tag has no effect in the trace's priority.

Plugin Checklist

Additional Notes

Copy link

github-actions bot commented Sep 30, 2024

Overall package size

Self size: 7.61 MB
Deduped: 64.47 MB
No deduping: 64.81 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.2.1 | 19.18 MB | 19.19 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.65 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.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 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

@iunanua iunanua changed the title Allow to set sampling priority Replace manual.keep tag usage with an specific method to keep the span Oct 1, 2024
@pr-commenter
Copy link

pr-commenter bot commented Oct 1, 2024

Benchmarks

Benchmark execution time: 2024-10-25 13:46:31

Comparing candidate commit 822d3ac in PR branch igor/set-sampling-priority with baseline commit f62cbfa in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 258 metrics, 8 unstable metrics.

@iunanua iunanua marked this pull request as ready for review October 14, 2024 13:04
@iunanua iunanua requested review from a team as code owners October 14, 2024 13:04
Copy link

codecov bot commented Oct 25, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.11%. Comparing base (7f93d36) to head (bfa5ae1).
Report is 42 commits behind head on master.

Files with missing lines Patch % Lines
packages/dd-trace/src/priority_sampler.js 85.71% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4739       +/-   ##
===========================================
+ Coverage   69.19%   80.11%   +10.92%     
===========================================
  Files           1      288      +287     
  Lines         198    12835    +12637     
  Branches       33        0       -33     
===========================================
+ Hits          137    10283    +10146     
- Misses         61     2552     +2491     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iunanua iunanua changed the title Replace manual.keep tag usage with an specific method to keep the span Replace manual.keep tag usage with an specific method to keep the trace Oct 25, 2024
@iunanua iunanua merged commit 4a711d9 into master Oct 29, 2024
207 checks passed
@iunanua iunanua deleted the igor/set-sampling-priority branch October 29, 2024 08:00
rochdev pushed a commit that referenced this pull request Oct 31, 2024
…ce (#4739)

* Allow to set sampling priority

* Span.keep method

* Replace manual.keep tag usage with Span.keep()

* Update standalone integration tests

* use PrioritySampler.keepTrace

* Lint

* PrioritySampler.keepTrace test
rochdev pushed a commit that referenced this pull request Oct 31, 2024
…ce (#4739)

* Allow to set sampling priority

* Span.keep method

* Replace manual.keep tag usage with Span.keep()

* Update standalone integration tests

* use PrioritySampler.keepTrace

* Lint

* PrioritySampler.keepTrace test
rochdev pushed a commit that referenced this pull request Oct 31, 2024
…ce (#4739)

* Allow to set sampling priority

* Span.keep method

* Replace manual.keep tag usage with Span.keep()

* Update standalone integration tests

* use PrioritySampler.keepTrace

* Lint

* PrioritySampler.keepTrace test
rochdev pushed a commit that referenced this pull request Nov 6, 2024
…ce (#4739)

* Allow to set sampling priority

* Span.keep method

* Replace manual.keep tag usage with Span.keep()

* Update standalone integration tests

* use PrioritySampler.keepTrace

* Lint

* PrioritySampler.keepTrace test
rochdev pushed a commit that referenced this pull request Nov 6, 2024
…ce (#4739)

* Allow to set sampling priority

* Span.keep method

* Replace manual.keep tag usage with Span.keep()

* Update standalone integration tests

* use PrioritySampler.keepTrace

* Lint

* PrioritySampler.keepTrace test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants