chore: Improving OpAmp telemetry#2594
Open
jordan-simonovski wants to merge 3 commits into
Open
Conversation
Improving some of our telemetry to make it easier to debug issues
🦋 Changeset detectedLatest commit: b178b96 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Contributor
Greptile SummaryThis PR turns the OpAMP message-handler span into a wide event by adding ~15 new attributes (agent identity, health, capability flags, config drift, request/response sizes) and fixes a latent bug where 64-bit protobuf fields (
Confidence Score: 5/5Safe to merge — changes are additive telemetry enrichment with no mutations to business logic, all new helpers are pure and covered by 11+ unit tests, and the error-middleware update is a classification-only change with a dedicated non-regression test. Every changed code path either adds new span attributes (additive, no side effects on request handling), fixes a silent data-loss bug in counter collection (Long coercion), or reclassifies a known benign error class. Test coverage is thorough and explicitly guards the prior-review concerns. No mutations to request routing, auth, or data persistence. No files require special attention. The pre-commit knip addition will add latency to every commit for all contributors, but this is intentional and documented. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[OpAMP POST /v1/messages] --> B{Buffer check}
B -->|yes| C[Set request.body_size_bytes]
B --> D[decodeAgentToServer]
C --> D
D --> E[Set instance_uid, sequence_num]
E --> F[Read health field]
F -->|present| G[Set healthy, health_last_error,\nhealth_status, uptime_ms]
F --> H[agentService.processAgentStatus]
G --> H
H --> I{isNewAgent?}
I -->|yes| J[Set is_new=true\ncounter: status=new]
I -->|no| K[Compute sequence_gap\nSet is_new=false\ncounter: status=updated]
J --> L{remoteConfigStatus\ntransition?}
K --> L
L -->|yes| M[opamp.remote_config_applications\ncounter += 1]
L --> N[Back to controller]
M --> N
N --> O[Set capabilities,\ncapability_flags]
O --> P[Flatten AgentDescription\nservice_name, service_version,\nos_type, host_arch]
P --> Q[Set remote_config_status,\nlast_remote_config_hash,\neffective_config attrs]
Q --> R{acceptsRemoteConfig?}
R -->|no| S[Set remote_config.sent=false]
R -->|yes| T[getAllTeams\nbuildOtelCollectorConfig\ncreateRemoteConfig]
T --> U[Set remote_config.sent=true\nremote_config.hash\nteams.count]
S --> V[encodeServerToAgent]
U --> V
V --> W[Set response.size_bytes\nspan OK\ncounter: outcome=processed]
subgraph Error Middleware
X[Express error] --> Y{bodyParserErrorType?}
Y -->|request.aborted| Z[clientDisconnect=true\noperational=true\nlog at debug\nskip recordException\nset received/expected bytes]
Y -->|other/none| AA{isOperationalError?}
AA -->|yes| AB[log at warn\nrecordException]
AA -->|no| AC[log at error\nrecordException]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[OpAMP POST /v1/messages] --> B{Buffer check}
B -->|yes| C[Set request.body_size_bytes]
B --> D[decodeAgentToServer]
C --> D
D --> E[Set instance_uid, sequence_num]
E --> F[Read health field]
F -->|present| G[Set healthy, health_last_error,\nhealth_status, uptime_ms]
F --> H[agentService.processAgentStatus]
G --> H
H --> I{isNewAgent?}
I -->|yes| J[Set is_new=true\ncounter: status=new]
I -->|no| K[Compute sequence_gap\nSet is_new=false\ncounter: status=updated]
J --> L{remoteConfigStatus\ntransition?}
K --> L
L -->|yes| M[opamp.remote_config_applications\ncounter += 1]
L --> N[Back to controller]
M --> N
N --> O[Set capabilities,\ncapability_flags]
O --> P[Flatten AgentDescription\nservice_name, service_version,\nos_type, host_arch]
P --> Q[Set remote_config_status,\nlast_remote_config_hash,\neffective_config attrs]
Q --> R{acceptsRemoteConfig?}
R -->|no| S[Set remote_config.sent=false]
R -->|yes| T[getAllTeams\nbuildOtelCollectorConfig\ncreateRemoteConfig]
T --> U[Set remote_config.sent=true\nremote_config.hash\nteams.count]
S --> V[encodeServerToAgent]
U --> V
V --> W[Set response.size_bytes\nspan OK\ncounter: outcome=processed]
subgraph Error Middleware
X[Express error] --> Y{bodyParserErrorType?}
Y -->|request.aborted| Z[clientDisconnect=true\noperational=true\nlog at debug\nskip recordException\nset received/expected bytes]
Y -->|other/none| AA{isOperationalError?}
AA -->|yes| AB[log at warn\nrecordException]
AA -->|no| AC[log at error\nrecordException]
end
Reviews (3): Last reviewed commit: "Merge branch 'main' into jordansimonovsk..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Deep Review✅ No critical issues found. Nothing in this diff is a guaranteed happy-path crash, data-loss, auth-bypass, or injection vector. The items below are recommended hardening and test/maintainability follow-ups. 🟡 P2 — recommended
🔵 P3 nitpicks (8)
Pre-existing (not introduced by this diff; not counted toward the verdict)
Reviewers (10): correctness, security, reliability, kieran-typescript, adversarial, testing, maintainability, project-standards, agent-native, learnings-researcher. Testing gaps: controller-level span enrichment and |
Contributor
E2E Test Results✅ All tests passed • 224 passed • 3 skipped • 1479s
Tests ran across 4 shards in parallel. |
teeohhem
reviewed
Jul 6, 2026
| periodic wide event — a heartbeat span carrying the readings — over a raw gauge, | ||
| so the readings stay sliceable; fall back to a gauge only when no such event | ||
| exists. | ||
|
|
teeohhem
reviewed
Jul 6, 2026
| engineering preference, not a rule against metrics — counters and histograms | ||
| remain first-class, feed alerts and SLOs, and many HyperDX deployments depend | ||
| heavily on them. See [Wide events over gauges](#wide-events-over-gauges). | ||
| 5. **Use the shared helpers** in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The OpAMP message handler is our own code in an observability product, but its span was effectively blind: it carried a single attribute (accepts_remote_config), while agent identity, capabilities, health, and config state lived only in debug logs or aggregate counters. When an agent misbehaves or a pushed config fails to apply, there was no way to pin the trace to a specific agent or slice by the dimensions that matter (collector version, OS, health). Separately, client disconnects mid-upload (request aborted) were being logged at error and counted as non-operational failures, inflating our error signal with benign background noise.
This PR turns the OpAMP span into a wide event and teaches the shared error middleware to classify body-parser errors correctly.
What changed
OpAMP handler span → wide event (opamp/controllers/opampController.ts, services/agentService.ts)
New pure helpers (opamp/utils/agentTelemetry.ts, unit-tested)
Error middleware (middleware/error.ts)
Docs (agent_docs/observability.md, AGENTS.md)
Testing
Notes / limitations