Skip to content

Node route harvesting - #924

Merged
grcevski merged 15 commits into
open-telemetry:mainfrom
grcevski:node_route_harvesting
Nov 26, 2025
Merged

grcevski merged 15 commits into
open-telemetry:mainfrom
grcevski:node_route_harvesting

Conversation

@grcevski

Copy link
Copy Markdown
Contributor

This PR extends our route harvesting capabilities to Node.js as well as the existing ones for Java and Go. Since Node.js applications are deployed as scripts, the code does a directory scan for a few of the most common Node.js web framewrorks and discovers routes used.

Supported frameworks:

  • Express (with route syntaxt too)
  • Fastify (short and long syntax)
  • Koa Router
  • Hapi
  • Restify
  • NestJS
  • HTTP Dispatcher
  • NextJS

The majority of the code are tests. Main logic of the harvesting is in js.go, with some extra helper libraries that help us determine what's the path for the main node.js script.

@grcevski
grcevski requested a review from a team as a code owner November 25, 2025 00:40
@codecov

codecov Bot commented Nov 25, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.59561% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.78%. Comparing base (d313973) to head (f2fc7f7).
⚠️ Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
pkg/internal/transform/route/harvest/js.go 92.03% 10 Missing and 10 partials ⚠️
pkg/internal/transform/route/harvest/harvester.go 76.47% 8 Missing ⚠️
pkg/ebpf/common/common_linux.go 94.11% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #924      +/-   ##
==========================================
+ Coverage   55.09%   55.78%   +0.68%     
==========================================
  Files         253      254       +1     
  Lines       21756    22085     +329     
==========================================
+ Hits        11987    12320     +333     
+ Misses       8945     8928      -17     
- Partials      824      837      +13     
Flag Coverage Δ
integration-test 22.82% <0.94%> (-0.36%) ⬇️
integration-test-arm 0.00% <0.00%> (ø)
integration-test-vm-${ARCH}-${KERNEL_VERSION} 0.00% <0.00%> (ø)
k8s-integration-test 2.68% <0.00%> (-0.05%) ⬇️
oats-test 0.00% <0.00%> (ø)
unittests 47.04% <90.59%> (+0.81%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@NimrodAvni78 NimrodAvni78 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Super great feature!

Comment thread pkg/internal/transform/route/harvest/harvester.go Outdated
Comment thread pkg/internal/transform/route/harvest/harvester.go Outdated
Comment thread pkg/internal/transform/route/harvest/harvester.go

@skl skl 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 for static routes

Comment thread pkg/internal/transform/route/harvest/js_test.go
@@ -58,10 +56,10 @@ func (e *HarvestError) Error() string {
func NewRouteHarvester(cfg *services.RouteHarvestingConfig, disabled []string, timeout time.Duration) *RouteHarvester {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@grcevski so the config is still a string[] but we compare it to the string enum?
maybe we can create MarshalText and UnmarshalText on InstrumentableType to allow users to specify it and have config type safety?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea! I will follow-up with a PR to fix this, either standalone or when I add ruby.

@NimrodAvni78 NimrodAvni78 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

small nit regarding the config, besides that LGTM

@grcevski
grcevski merged commit 4dfa4f1 into open-telemetry:main Nov 26, 2025
54 checks passed
@grcevski
grcevski deleted the node_route_harvesting branch November 26, 2025 16:00
@MrAlias MrAlias added this to the v0.3.0 milestone Dec 3, 2025
@MrAlias MrAlias mentioned this pull request Dec 3, 2025
marctc pushed a commit to grafana/opentelemetry-ebpf-instrumentation that referenced this pull request Dec 9, 2025
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.

4 participants