feat(apm): editor APM enrichment service + tRPC route#2838
Open
jonmcwest wants to merge 1 commit into
Open
Conversation
2 tasks
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
2 tasks
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/workspace-server/src/services/apm-enrichment/apmEnrichment.ts:91-96
The access-token fetch failure logs at `debug` while the expected "not yet authenticated" path logs at `info`. If `getValidAccessToken` ever throws unexpectedly (network error, token store corrupted, etc.) the signal will be buried below the noise of normal debug output and be very hard to diagnose. `warn` is a more appropriate level for an unplanned exception.
```suggestion
} catch (err) {
this.log.warn("Failed to resolve access token", {
message: err instanceof Error ? err.message : String(err),
});
return null;
}
```
### Issue 2 of 2
packages/workspace-server/src/services/apm-enrichment/apmEnrichment.test.ts:71-85
**Tests prefer parameterized form for similar scenarios**
The "returns null" cases (unauthenticated at line 71 and HTTP 500 at line 156) test the same observable outcome with different failure triggers. Per the team's preference for parameterised tests, these are good candidates for an `it.each` table, e.g. pairing `(description, authFactory, fetchResponse)` rows. In addition, the `catch` branch in `resolveApiConfig` — when `getValidAccessToken()` itself throws — has no coverage at all, which would be a natural third row in such a table.
Reviews (1): Last reviewed commit: "feat(apm): editor APM enrichment service..." | Re-trigger Greptile |
696d183 to
d02ea61
Compare
502d78c to
214b340
Compare
2 tasks
214b340 to
df9dcd6
Compare
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
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.

Problem
The editor has no way to surface APM (Application Performance Monitoring) data alongside source files. Users viewing a file in the editor cannot see per-line tracing stats such as call counts, error rates, or latency percentiles without leaving the editor and navigating to the PostHog tracing explorer.
Changes
Introduced a new
ApmEnrichmentServicethat queries the PostHogtracing/spans/symbol-stats/endpoint in line-granularity mode for a given file path. The service reuses the existingENRICHMENT_AUTHport for PostHog API credentials and active project resolution, so no new authentication wiring is needed.ApmEnrichmentServicewith anenrichFilemethod that returns per-line stats (count,errorCount,p50Ms,p95Ms, percentage changes) and atracingUrlpointing to the PostHog tracing explorer, ornullwhen unauthenticated or when the query fails.apmEnrichmentModuleInversifyContainerModuleand loaded it into the main DI container.apmEnrichment.enrichFilequery procedure, registered on both the host router and the main app router.APM_ENRICHMENT_SERVICEas a typed DI identifier and declared the binding inMainBindings.How did you test this?
Added unit tests in
apmEnrichment.test.tscovering:nulland makes no network request when the user is unauthenticated.TraceSpansSymbolStatsQueryto the right endpoint with the rightAuthorizationheader and date range.SerializedApmEnrichmentshape, including nanosecond-to-millisecond conversion and percentage change fields.nullwhen the symbol-stats endpoint responds with a non-OK status.Automatic notifications