Skip to content

Conversation

szegedi
Copy link
Contributor

@szegedi szegedi commented Jul 20, 2023

Features

Enhancements

Bug fixes

Minor stuff (probably not notable for release notes)

nsavoire and others added 22 commits July 20, 2023 16:51
…ler (#3371)

* Initial code for code hotspots and endpoint aggregation in wall profiler

Hide functionality behind DD_PROFILING_EXPERIMENTAL_CODEHOTSPOTS_ENABLED
and DD_PROFILING_EXPERIMENTAL_ENDPOINT_COLLECTION_ENABLED flags.

Profiler tracks changes of active span by subscribing to async hooks
before channel and to AsyncResourceStorage enter channel that
notifies when current store is changed with enterWith/run.
Profiler remembers previous span and previous started spans, upon change
of active span, it checks if a sample has been taken by native profiler
addon, then if that's the case, it updates the context of the sample
with span id and root span id computed from previous span / started spans.
It also add the tags from the last web span to the context to allow
computation of endpoint later on.

Minor changes to other parts of the code to make it all work:
===

* Report error if start fails

* Avoid clearing span context tags upon export

Profiler may need the tags to determine span resource name for endpoint
aggregation: profiler keeps a reference on web span that was active when
each profiling sample is taken. Then during profile export, span tags
are used to determine endpoint. Endpoint cannot be determined right away
because sometimes tags necessary to determine endpoint are only set just
before span is finished.

* Add request tags just after span creation

For web spans, add request tags just after span creation instead of when
span finishes. This is needed for profiler code hotspots implementation
to determine which span are web spans and should be used to find current
endpoint.
* Taint request URI

* Add check for safe tainted origins on unvalidated redirect analyzer

* Change assertion construction for unvalidated redirect analyzer test

* Add metric for uri sourcing

* Fix PR comments
* Detect X-Content-Type-Options missing header

* HSTS Header missing analyzer and refactor of xcontenttype-header-missing-aanalyzer

* Move function to class method

* use startswith instead of index===0

* Do not send evidence if value is undefined

* Fix comment in PR and add test

* Changes to support telemetry

* Rename method name

* Rename analyzer object key
Forgot to update yarn.lock when bumping pprof-nodejs version in a
previous PR.
* Fix same dependency detection

* Do not send same dependency:version twice

* Clear map on stop

* Update savedDependencies to savedDependenciesToSend to improve understanding
* address and fix edge cases
* move to better tracer option name for service name flattening
* implement naming schema for graphql plugin
* implement schema v1 and add tests

---------

Co-authored-by: Thomas Hunter II <[email protected]>
Co-authored-by: Jordi Bertran de Balanda <[email protected]>
@szegedi szegedi requested review from a team as code owners July 20, 2023 14:55
@szegedi szegedi changed the base branch from master to v2.x July 20, 2023 14:56
@pr-commenter
Copy link

pr-commenter bot commented Jul 20, 2023

Benchmarks

Benchmark execution time: 2023-07-20 15:08:34

Comparing candidate commit 5ff0238 in PR branch v2.42.0-proposal with baseline commit 4b02e9e in branch v2.x.

Found 1 performance improvements and 32 performance regressions! Performance is the same for 667 metrics, 35 unstable metrics.

scenario:appsec-iast-no-vulnerability-control-14

  • 🟥 cpu_usage_percentage [+5.520%; +8.264%]

scenario:appsec-iast-no-vulnerability-control-16

  • 🟥 cpu_usage_percentage [+5.776%; +8.071%]

scenario:exporting-pipeline-0.4-12

  • 🟥 cpu_user_time [+59.207ms; +64.426ms] or [+24.600%; +26.768%]
  • 🟥 execution_time [+61.821ms; +64.792ms] or [+22.451%; +23.530%]
  • 🟥 instructions [+120.0M instructions; +120.7M instructions] or [+16.274%; +16.362%]
  • 🟥 max_rss_usage [+3.207KB; +3.445KB] or [+5.940%; +6.380%]

scenario:exporting-pipeline-0.4-14

  • 🟥 cpu_user_time [+49.063ms; +54.380ms] or [+19.566%; +21.687%]
  • 🟥 execution_time [+53.020ms; +57.116ms] or [+18.635%; +20.074%]
  • 🟥 instructions [+120.3M instructions; +121.5M instructions] or [+16.081%; +16.251%]
  • 🟥 max_rss_usage [+5.331KB; +5.822KB] or [+10.579%; +11.552%]

scenario:exporting-pipeline-0.4-16

  • 🟥 cpu_user_time [+40.673ms; +45.816ms] or [+16.561%; +18.656%]
  • 🟥 execution_time [+42.339ms; +47.051ms] or [+15.110%; +16.792%]
  • 🟥 instructions [+98.9M instructions; +101.0M instructions] or [+13.600%; +13.892%]
  • 🟥 max_rss_usage [+4.022KB; +4.074KB] or [+7.217%; +7.311%]

scenario:exporting-pipeline-0.4_with_stats-12

  • 🟥 cpu_user_time [+58.673ms; +64.057ms] or [+24.441%; +26.683%]
  • 🟥 execution_time [+60.864ms; +64.115ms] or [+21.924%; +23.095%]
  • 🟥 instructions [+119.8M instructions; +120.5M instructions] or [+16.172%; +16.260%]
  • 🟥 max_rss_usage [+2.880KB; +3.024KB] or [+5.337%; +5.603%]

scenario:exporting-pipeline-0.4_with_stats-14

  • 🟥 cpu_user_time [+47.610ms; +53.143ms] or [+18.750%; +20.930%]
  • 🟥 execution_time [+54.106ms; +58.128ms] or [+18.938%; +20.345%]
  • 🟥 instructions [+120.3M instructions; +121.6M instructions] or [+16.000%; +16.175%]
  • 🟥 max_rss_usage [+5.136KB; +5.525KB] or [+10.227%; +11.002%]

scenario:exporting-pipeline-0.4_with_stats-16

  • 🟥 cpu_user_time [+48.329ms; +53.854ms] or [+20.323%; +22.646%]
  • 🟥 execution_time [+53.957ms; +58.579ms] or [+19.851%; +21.551%]
  • 🟥 instructions [+119.4M instructions; +122.5M instructions] or [+16.774%; +17.205%]
  • 🟥 max_rss_usage [+3.925KB; +4.004KB] or [+7.032%; +7.172%]

scenario:exporting-pipeline-0.5-12

  • 🟥 instructions [+50.7M instructions; +51.0M instructions] or [+8.250%; +8.294%]

scenario:exporting-pipeline-0.5-14

  • 🟥 instructions [+46.6M instructions; +46.9M instructions] or [+7.654%; +7.716%]

scenario:exporting-pipeline-0.5-16

  • 🟥 instructions [+52.2M instructions; +54.9M instructions] or [+8.847%; +9.291%]

scenario:exporting-pipeline-0.5_with_stats-12

  • 🟥 instructions [+50.9M instructions; +51.2M instructions] or [+8.240%; +8.282%]

scenario:exporting-pipeline-0.5_with_stats-14

  • 🟥 instructions [+46.3M instructions; +46.6M instructions] or [+7.578%; +7.620%]

scenario:exporting-pipeline-0.5_with_stats-16

  • 🟥 instructions [+53.1M instructions; +55.6M instructions] or [+8.960%; +9.374%]

scenario:spans-finish-later-14

  • 🟩 max_rss_usage [-10.473KB; -10.174KB] or [-5.201%; -5.053%]

@szegedi
Copy link
Contributor Author

szegedi commented Jul 20, 2023

Closing the PR as checks failed to initialize properly

@szegedi szegedi closed this Jul 20, 2023
@szegedi szegedi deleted the v2.42.0-proposal branch July 20, 2023 15:41
@szegedi szegedi restored the v2.42.0-proposal branch July 20, 2023 15:41
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.

10 participants