Skip to content

(DynamicSelector-Phase4) Propagate signal ownership through app discovery and lifecycle consumers#2421

Open
damemi wants to merge 2 commits into
open-telemetry:mainfrom
damemi:dynamic-signals-phase4
Open

(DynamicSelector-Phase4) Propagate signal ownership through app discovery and lifecycle consumers#2421
damemi wants to merge 2 commits into
open-telemetry:mainfrom
damemi:dynamic-signals-phase4

Conversation

@damemi

@damemi damemi commented Jun 17, 2026

Copy link
Copy Markdown
Member

This implements phase 3 of #2234

Previous PRs:

  1. (DynamicSelector-Phase1) Move DynamicPIDSelector to global context #2250
  2. (DynamicSelector-Phase2) Extend dynamic selection to network and stats #2279
  3. (DynamicSelector-Phase3) Refactor DynamicPIDSelector into signal-aware views #2340

This PR wires up the actual per-signal ownership for AppO11y, Network, and Stats Metrics. AppO11y still uses a union view for app traces+metrics (will be split in the next PR). Dynamic/Parent pid tracking is added for appolly/app/svc/svc.go and discover/matcher.go with the DynamicSelectorPID field.

Summary

Describe the change briefly.

Validation

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.18182% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.27%. Comparing base (db7eae0) to head (e83774c).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
pkg/netolly/agent/pipeline.go 40.00% 2 Missing and 1 partial ⚠️
pkg/statsolly/agent/pipeline.go 40.00% 2 Missing and 1 partial ⚠️
pkg/appolly/discover/finder.go 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2421      +/-   ##
==========================================
+ Coverage   67.77%   69.27%   +1.50%     
==========================================
  Files         320      332      +12     
  Lines       42409    44207    +1798     
==========================================
+ Hits        28741    30624    +1883     
+ Misses      12043    11690     -353     
- Partials     1625     1893     +268     
Flag Coverage Δ
integration-test 51.05% <36.84%> (+3.78%) ⬆️
integration-test-arm 28.87% <13.63%> (+0.65%) ⬆️
integration-test-vm-5.15-lts ?
integration-test-vm-6.18-lts ?
k8s-integration-test 34.93% <26.31%> (-2.95%) ⬇️
oats-test 36.49% <15.78%> (+<0.01%) ⬆️
unittests 62.55% <78.94%> (+0.54%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

@mariomac mariomac 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.

LGTM!

@grcevski grcevski 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.

LGTM!

Copilot AI 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.

Pull request overview

This PR advances the DynamicSelector “signal ownership” work by wiring signal-specific PID selector views into the NetO11y and StatsO11y pipelines, and by starting to propagate an “owner PID” (DynamicSelectorPID) through AppO11y discovery into service attributes for downstream lifecycle consumers.

Changes:

  • Wire DynamicPIDSelector.NetworkMetrics() and .StatsMetrics() into the NetO11y/StatsO11y filter.ByDynamicPID stage to support per-signal PID ownership.
  • Add DynamicSelectorPID to AppO11y discovery match results (ProcessMatch) and propagate it into svc.Attrs.
  • Update AppO11y dynamic matcher plumbing to use the app-signals selector view (appSignals()) and add a basic assertion for DynamicSelectorPID in tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/statsolly/agent/pipeline.go Switch StatsO11y dynamic PID filtering to the StatsMetrics() selector view.
pkg/netolly/agent/pipeline.go Switch NetO11y dynamic PID filtering to the NetworkMetrics() selector view.
pkg/appolly/discover/typer.go Propagate ProcessMatch.DynamicSelectorPID into svc.Attrs.
pkg/appolly/discover/matcher.go Extend ProcessMatch to carry DynamicSelectorPID for ownership tracking.
pkg/appolly/discover/matcher_test.go Update dynamic matcher tests to use the app-signals view and assert DynamicSelectorPID.
pkg/appolly/discover/matcher_dynamic.go Change dynamic matcher to operate on *dynamicPIDSignalView and set DynamicSelectorPID on dynamic matches.
pkg/appolly/discover/finder.go Wire the app-signals dynamic selector view through finder components (process watcher + dynamic matcher).
pkg/appolly/app/svc/svc.go Add DynamicSelectorPID to service attributes to support lifecycle ownership propagation.

Comment thread pkg/appolly/discover/matcher.go
Comment thread pkg/appolly/discover/matcher_test.go
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