feat(controlplane): add service-level tracing spans to Connect-RPC handlers#2771
Merged
thisisnithin merged 5 commits intomainfrom Apr 17, 2026
Conversation
…ndlers Add @Traced decorator and Connect interceptor to propagate Sentry spans through Connect-RPC handlers for full visibility into repository and service method timings. Resolves ENG-9385
thisisnithin
requested review from
a team,
JivusAyrus,
StarpTech and
wilsonrivera
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds Sentry tracing primitives and integration: a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (65.56%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2771 +/- ##
==========================================
+ Coverage 40.72% 47.36% +6.64%
==========================================
Files 985 1062 +77
Lines 123243 143591 +20348
Branches 5639 9759 +4120
==========================================
+ Hits 50187 68008 +17821
- Misses 71381 73822 +2441
- Partials 1675 1761 +86
🚀 New features to boost your workflow:
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
controlplane/src/core/services/RBACEvaluator.ts (1)
3-26: Consider not tracingRBACEvaluatorto avoid span explosion on hot permission checks.This class has many tiny synchronous methods; decorating the whole class can produce noisy traces and unnecessary overhead.
♻️ Suggested adjustment
import { OrganizationRole } from '../../db/models.js'; import { OrganizationGroupDTO } from '../../types/index.js'; -import { traced } from '../tracing.js'; @@ -@traced export class RBACEvaluator {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/services/RBACEvaluator.ts` around lines 3 - 26, The RBACEvaluator class is fully decorated with `@traced` which causes span explosion for many tiny synchronous permission-check methods; remove the class-level `@traced` decorator from the RBACEvaluator declaration and instead add tracing only to the handful of genuinely heavy or async methods (if any) by applying traced to specific methods (e.g., longRunningCheck, evaluatePolicy, or any async entrypoints) or wrapping their bodies with traced() calls so hot-path sync helpers remain uninstrumented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/repositories/FederatedGraphRepository.ts`:
- Line 77: composeAndDeployGraphs is currently defined as a class-field arrow
function so the traced decorator (imported as traced) cannot wrap it; change
composeAndDeployGraphs from an instance arrow property to a normal prototype
method (e.g., remove the "composeAndDeployGraphs = async (...) => { ... }"
class-field form and declare "async composeAndDeployGraphs(...) { ... }"),
ensure any uses relying on lexical this remain valid (bind callers if needed)
and keep the traced decorator application logic unchanged so traced will now
instrument the method on the prototype.
In `@controlplane/src/core/services/ApiGenerator.ts`:
- Around line 4-7: The traced decorator is only wrapping prototype methods so
the static ApiKeyGenerator.generate() is never instrumented; update the
decorator to also iterate Object.getOwnPropertyNames(target) (the constructor)
in addition to Object.getOwnPropertyNames(target.prototype), skipping
"prototype" and "constructor" and only wrapping function-valued properties,
preserving property descriptors and not re-wrapping already-wrapped functions;
this will ensure static methods like ApiKeyGenerator.generate are traced without
changing the class API.
In `@controlplane/src/core/services/BillingService.ts`:
- Around line 14-15: BillingService contains arrow-function class fields
(notably upsertStripeCustomerId and cancelSubscription) that are instance
properties and therefore not covered by the `@traced` decorator; convert these
arrow-field definitions into standard prototype methods so they are
instrumented. Specifically, change declarations like "upsertStripeCustomerId =
async (...) => { ... }" to "async upsertStripeCustomerId(...) { ... }" (and
similarly for cancelSubscription), keep the same parameter list and body, and if
any callers rely on function identity as a bound callback, bind the methods in
the constructor (e.g., this.upsertStripeCustomerId =
this.upsertStripeCustomerId.bind(this)) or update call sites accordingly. Ensure
no other arrow-function class fields remain that should be traced so `@traced` can
instrument the prototype methods.
In `@controlplane/src/core/services/SchemaGraphPruner.ts`:
- Around line 20-22: The `@traced` decorator is ineffective because it only
instruments prototype methods but SchemaGraphPruner defines methods as
class-field arrow functions (e.g., getAllFields = (...) => { ... }), so either
remove the `@traced` decorator or convert those arrow functions to prototype
methods; locate the SchemaGraphPruner class and either delete the `@traced`
import/annotation or refactor instance properties like getAllFields (and any
other arrow methods) into standard method declarations (e.g., getAllFields(...)
{ ... }) so the tracer can wrap them.
In `@controlplane/src/core/services/SchemaLinter.ts`:
- Line 13: The methods getRuleModule, createRulesConfig, and schemaLintCheck are
defined as class field arrow functions so the `@traced` decorator (which wraps
prototype methods by iterating Object.getOwnPropertyNames(proto)) cannot wrap
them; change each from an instance property arrow (e.g., getRuleModule = () => {
... }) to a prototype method (e.g., getRuleModule(...) { ... }) so they live on
the prototype and will be instrumented by traced; if any of these relied on
lexical this-binding, add explicit binding in the constructor or refactor
callers to call the prototype methods directly, and keep the traced import/usage
intact.
In `@controlplane/src/core/tracing.ts`:
- Around line 34-35: withSpan currently declares Promise<T> return but calls
Sentry.startSpan which may return the callback's raw value; this can return a
non-Promise at runtime. Change withSpan to be async (export async function
withSpan<T>(...)) so it always returns a Promise, remove the unsafe cast to
Promise<T>, and return the result of await Sentry.startSpan({ name }, () =>
fn()) (or simply await the callback inside the async function) so the signature
and runtime behavior match; update any usages if necessary.
In `@controlplane/src/core/util.ts`:
- Around line 100-113: The current code builds a single sentryContext with
namespaced keys and passes it to Sentry.setUser, which prevents Sentry from
recognizing canonical user fields; instead create two objects: a flat
userContext using authContext.userId -> id, authContext.userDisplayName ->
username (or displayName), and any email/ip fields, and pass that to
Sentry.setUser(userContext); keep the namespaced sentryAttributes (e.g.,
'user.id', 'user.displayName', 'organization.id', 'organization.slug') and pass
those to Sentry.getRootSpan(activeSpan).setAttributes(sentryAttributes); update
the code around sentryContext, authContext, Sentry.setUser, Sentry.getActiveSpan
and Sentry.getRootSpan accordingly.
---
Nitpick comments:
In `@controlplane/src/core/services/RBACEvaluator.ts`:
- Around line 3-26: The RBACEvaluator class is fully decorated with `@traced`
which causes span explosion for many tiny synchronous permission-check methods;
remove the class-level `@traced` decorator from the RBACEvaluator declaration and
instead add tracing only to the handful of genuinely heavy or async methods (if
any) by applying traced to specific methods (e.g., longRunningCheck,
evaluatePolicy, or any async entrypoints) or wrapping their bodies with traced()
calls so hot-path sync helpers remain uninstrumented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50eb046c-fa82-4c99-9c7d-0b3ac59cd1b9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
controlplane/package.jsoncontrolplane/src/core/auth-utils.tscontrolplane/src/core/blobstorage/dual.tscontrolplane/src/core/blobstorage/s3.tscontrolplane/src/core/build-server.tscontrolplane/src/core/clickhouse/client/ClickHouseClient.tscontrolplane/src/core/composition/composer.tscontrolplane/src/core/repositories/ApiKeyRepository.tscontrolplane/src/core/repositories/AuditLogRepository.tscontrolplane/src/core/repositories/BillingRepository.tscontrolplane/src/core/repositories/CacheWarmerRepository.tscontrolplane/src/core/repositories/ContractRepository.tscontrolplane/src/core/repositories/FeatureFlagRepository.tscontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/GitHubRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/src/core/repositories/NamespaceRepository.tscontrolplane/src/core/repositories/OidcRepository.tscontrolplane/src/core/repositories/OperationsRepository.tscontrolplane/src/core/repositories/OrganizationGroupRepository.tscontrolplane/src/core/repositories/OrganizationInvitationRepository.tscontrolplane/src/core/repositories/OrganizationRepository.tscontrolplane/src/core/repositories/PlaygroundScriptsRepository.tscontrolplane/src/core/repositories/PluginRepository.tscontrolplane/src/core/repositories/ProposalRepository.tscontrolplane/src/core/repositories/SchemaCheckRepository.tscontrolplane/src/core/repositories/SchemaGraphPruningRepository.tscontrolplane/src/core/repositories/SchemaLintRepository.tscontrolplane/src/core/repositories/SubgraphCheckExtensionsRepository.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/repositories/TargetRepository.tscontrolplane/src/core/repositories/UserRepository.tscontrolplane/src/core/repositories/analytics/AnalyticsDashboardViewRepository.tscontrolplane/src/core/repositories/analytics/AnalyticsRequestViewRepository.tscontrolplane/src/core/repositories/analytics/MetricsRepository.tscontrolplane/src/core/repositories/analytics/MonthlyRequestViewRepository.tscontrolplane/src/core/repositories/analytics/RouterMetricsRepository.tscontrolplane/src/core/repositories/analytics/SubgraphMetricsRepository.tscontrolplane/src/core/repositories/analytics/TraceRepository.tscontrolplane/src/core/repositories/analytics/UsageRepository.tscontrolplane/src/core/sentry.config.tscontrolplane/src/core/services/AccessTokenAuthenticator.tscontrolplane/src/core/services/AdmissionWebhookController.tscontrolplane/src/core/services/ApiGenerator.tscontrolplane/src/core/services/ApiKeyAuthenticator.tscontrolplane/src/core/services/ApolloMigrator.tscontrolplane/src/core/services/Authentication.tscontrolplane/src/core/services/Authorization.tscontrolplane/src/core/services/BillingService.tscontrolplane/src/core/services/GraphApiTokenAuthenticator.tscontrolplane/src/core/services/Keycloak.tscontrolplane/src/core/services/Mailer.tscontrolplane/src/core/services/OidcProvider.tscontrolplane/src/core/services/RBACEvaluator.tscontrolplane/src/core/services/SchemaGraphPruner.tscontrolplane/src/core/services/SchemaLinter.tscontrolplane/src/core/services/SchemaUsageTrafficInspector.tscontrolplane/src/core/services/Slack.tscontrolplane/src/core/services/UserInviteService.tscontrolplane/src/core/services/WebSessionAuthenticator.tscontrolplane/src/core/services/WorkspaceService.tscontrolplane/src/core/tracing.tscontrolplane/src/core/util.tscontrolplane/src/core/webhooks/OrganizationWebhookService.tscontrolplane/src/core/webhooks/PlatformWebhookService.tscontrolplane/src/core/webhooks/RedeliverWebhookService.tscontrolplane/tsconfig.json
- Fix @Traced decorator to also wrap arrow-function class fields - Fix Sentry.setUser to use canonical fields (id, username) - Keep span attributes with dotted names for trace filtering
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
controlplane/src/core/util.ts (2)
78-78: Rename the exported symbol constant toUPPER_SNAKE_CASE.The new symbol follows the existing local pattern, but it still violates the repo rule for constants. Prefer
SENTRY_SPAN_IDhere to avoid baking in more mixed naming.As per coding guidelines,
**/*.{js,jsx,ts,tsx}: Use UPPER_SNAKE_CASE for constants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/util.ts` at line 78, Rename the exported constant symbol from sentrySpanId to SENTRY_SPAN_ID: change the export declaration (export const sentrySpanId = Symbol('sentrySpan')) to export const SENTRY_SPAN_ID = Symbol('sentrySpan') and update every usage/import that references sentrySpanId to use SENTRY_SPAN_ID instead (search for sentrySpanId in references, tests, and consumers). Ensure TypeScript types and any destructuring or re-exports adapt to the new identifier so builds and imports continue to work.
84-88: Add an explicit return type toenrichLogger.This helper is now doing more than simple logger decoration. Locking the return type to
FastifyBaseLoggerkeeps the utility contract stable if this body changes again.Suggested change
export const enrichLogger = ( ctx: HandlerContext, logger: FastifyBaseLogger, authContext: Partial<AuthContext & GraphKeyAuthContext>, -) => { +): FastifyBaseLogger => {As per coding guidelines,
**/*.{ts,tsx}: Use explicit type annotations for function parameters and return types in TypeScript.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/util.ts` around lines 84 - 88, The enrichLogger function lacks an explicit return type; update its signature to declare the return type as FastifyBaseLogger to satisfy the typing guideline and lock the utility contract (i.e., change the function declaration of enrichLogger(ctx: HandlerContext, logger: FastifyBaseLogger, authContext: Partial<AuthContext & GraphKeyAuthContext>): FastifyBaseLogger) so consumers and future refactors cannot accidentally change the exposed type; ensure the signature uses the existing symbol names (enrichLogger, HandlerContext, FastifyBaseLogger, AuthContext, GraphKeyAuthContext) and that any related exports still compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/util.ts`:
- Around line 100-117: Wrap the Sentry enrichment block in a try-catch so
failures don't bubble to the request path; inside the try, build a user object
by picking/filtering defined fields from authContext.userId and
authContext.userDisplayName and only call Sentry.setUser(user) if that object
has keys, otherwise call Sentry.setUser(null) to clear user context; similarly
build spanAttributes by filtering undefined values (as currently done) and only
call
Sentry.getActiveSpan()/Sentry.getRootSpan(activeSpan).setAttributes(spanAttributes)
if an active span exists; in the catch, log the error as a best-effort telemetry
enrichment and do not rethrow.
---
Nitpick comments:
In `@controlplane/src/core/util.ts`:
- Line 78: Rename the exported constant symbol from sentrySpanId to
SENTRY_SPAN_ID: change the export declaration (export const sentrySpanId =
Symbol('sentrySpan')) to export const SENTRY_SPAN_ID = Symbol('sentrySpan') and
update every usage/import that references sentrySpanId to use SENTRY_SPAN_ID
instead (search for sentrySpanId in references, tests, and consumers). Ensure
TypeScript types and any destructuring or re-exports adapt to the new identifier
so builds and imports continue to work.
- Around line 84-88: The enrichLogger function lacks an explicit return type;
update its signature to declare the return type as FastifyBaseLogger to satisfy
the typing guideline and lock the utility contract (i.e., change the function
declaration of enrichLogger(ctx: HandlerContext, logger: FastifyBaseLogger,
authContext: Partial<AuthContext & GraphKeyAuthContext>): FastifyBaseLogger) so
consumers and future refactors cannot accidentally change the exposed type;
ensure the signature uses the existing symbol names (enrichLogger,
HandlerContext, FastifyBaseLogger, AuthContext, GraphKeyAuthContext) and that
any related exports still compile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73af4442-8f40-452b-a090-31118770caed
📒 Files selected for processing (2)
controlplane/src/core/tracing.tscontrolplane/src/core/util.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- controlplane/src/core/tracing.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
controlplane/test/tracing.test.ts (1)
136-147: Strengthen thetraced()function-call test by using the returned class.The current test passes even if only prototype mutation works. Assign and instantiate the returned class so this case validates function-call behavior more directly (especially useful for constructor-time wrapping semantics).
Suggested diff
- traced(ManualService); - - const svc = new ManualService(); + const TracedManualService = traced(ManualService); + const svc = new TracedManualService(); expect(svc.doWork()).toBe('done');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/test/tracing.test.ts` around lines 136 - 147, The test currently calls traced(ManualService) but doesn't use its return value, which misses validating the function-call form; update the test to capture the returned class from traced (e.g., const Wrapped = traced(ManualService)) and instantiate that returned class (new Wrapped()) instead of the original ManualService, then assert the instance.doWork() result and that startSpanMock was called with { name: 'ManualService.doWork' } and a function; this ensures traced()'s returned-class wrapping (constructor-time semantics) is exercised rather than relying on prototype mutation alone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/test/tracing.test.ts`:
- Around line 21-23: Several class methods in controlplane/test/tracing.test.ts
(for example the greet(name: string) method) are missing explicit return type
annotations; update each class method declaration to include an explicit return
type (e.g., greet(name: string): string) so every method in the class has a
declared return type for TypeScript consistency—locate methods such as greet and
the other methods referenced in the review and add the appropriate return types
(string, void, Promise<...>, etc.) matching their returned values or promises.
- Line 7: The mock for startSpan should stop using any and match Sentry's API:
change the mock signature in the startSpan vi.fn to use unknown types and a
no-argument callback—e.g., replace (opts: { name: string }, cb: (span: any) =>
any) => cb({ name: opts.name }) with a signature using unknown like (opts: {
name: string }, cb: () => unknown) => cb() so the callback takes no parameters
and the types avoid any.
---
Nitpick comments:
In `@controlplane/test/tracing.test.ts`:
- Around line 136-147: The test currently calls traced(ManualService) but
doesn't use its return value, which misses validating the function-call form;
update the test to capture the returned class from traced (e.g., const Wrapped =
traced(ManualService)) and instantiate that returned class (new Wrapped())
instead of the original ManualService, then assert the instance.doWork() result
and that startSpanMock was called with { name: 'ManualService.doWork' } and a
function; this ensures traced()'s returned-class wrapping (constructor-time
semantics) is exercised rather than relying on prototype mutation alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ad46991-75bd-4625-ade2-0df7a6c6101a
📒 Files selected for processing (1)
controlplane/test/tracing.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
controlplane/test/tracing.test.ts (2)
7-7:⚠️ Potential issue | 🟡 MinorReplace
anyin the Sentry mock signature.Use
unknown-based types to keep the mock type-safe.Suggested diff
- startSpan: vi.fn((opts: { name: string }, cb: (span: any) => any) => cb({ name: opts.name })), + startSpan: vi.fn( + (opts: { name: string }, cb: (span: unknown) => unknown): unknown => cb({ name: opts.name }), + ),As per coding guidelines, "Avoid
anytype in TypeScript; use specific types or generics."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/test/tracing.test.ts` at line 7, The test Sentry mock uses `any` in the startSpan signature; update the mock in the startSpan vi.fn to use unknown-based types for safety: change the callback type from `(span: any) => any` to `(span: unknown) => unknown` (or a more specific generic/opaque type if available) and cast the object passed to the callback (`{ name: opts.name }`) to unknown when invoking cb; this keeps startSpan (the vi.fn mock) type-safe while preserving behavior in tracing.test.ts.
21-23:⚠️ Potential issue | 🟡 MinorAdd explicit return types to test class methods.
These methods currently rely on inference; add explicit returns to comply with TS guidelines.
Suggested diff (examples)
- greet(name: string) { + greet(name: string): string { return `hello ${name}`; } - findById(id: string) { + findById(id: string): Promise<{ id: string; name: string }> { return Promise.resolve({ id, name: 'test' }); } - increment() { + increment(): number { this.count += 1; return this.count; } - getValue() { + getValue(): string { return this.name; } - doWork() { + doWork(): string { return 'done'; }As per coding guidelines, "Use explicit type annotations for function parameters and return types in TypeScript."
Also applies to: 36-38, 53-56, 70-72, 92-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/test/tracing.test.ts` around lines 21 - 23, Add explicit return type annotations for the test class methods (e.g., change greet(name: string) { ... } to include a return type like greet(name: string): string { ... }) and do the same for the other methods flagged in this file (the methods at the other ranges referenced in the review). Locate the methods by their names (e.g., greet and the other test helpers/assertions in the same test class) and annotate each function with an explicit return type consistent with the returned value (string, void, boolean, Promise<...>, etc.) to satisfy the TypeScript guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/eslint-local-rules.cjs`:
- Around line 8-13: The isTraced helper currently assumes node.parent.body
exists and therefore misses classes wrapped in ExportDefaultDeclaration (e.g.,
"export default class ...; traced(Class)"). Update isTraced to detect when the
node's parent is an ExportDefaultDeclaration and, in that case, examine
parent.parent.body (or the nearest ancestor with a body array) for siblings;
otherwise continue using parent.body. Specifically modify the sibling lookup in
isTraced (the parent/body/siblings/indexOf logic) to fall back to the export's
parent body so exported classes are included in the traced-class sibling scan.
In `@controlplane/src/core/tracing.ts`:
- Around line 3-5: The decorator helper wrapMethod currently uses weak types
(Function, any) — change its signature to be generic and unknown-safe: make
wrapMethod a generic function (e.g., <TArgs extends unknown[], TResult>) and
type the original parameter as (...args: TArgs) => TResult and the returned
wrapper as (this: any, ...args: TArgs) => TResult (also replace any[]/any in the
rest of the function with TArgs and unknown/ TResult accordingly); update the
other similar helper at the noted location (line ~17) to follow the same pattern
so that arguments and return values are strongly typed without using any or
Function.
---
Duplicate comments:
In `@controlplane/test/tracing.test.ts`:
- Line 7: The test Sentry mock uses `any` in the startSpan signature; update the
mock in the startSpan vi.fn to use unknown-based types for safety: change the
callback type from `(span: any) => any` to `(span: unknown) => unknown` (or a
more specific generic/opaque type if available) and cast the object passed to
the callback (`{ name: opts.name }`) to unknown when invoking cb; this keeps
startSpan (the vi.fn mock) type-safe while preserving behavior in
tracing.test.ts.
- Around line 21-23: Add explicit return type annotations for the test class
methods (e.g., change greet(name: string) { ... } to include a return type like
greet(name: string): string { ... }) and do the same for the other methods
flagged in this file (the methods at the other ranges referenced in the review).
Locate the methods by their names (e.g., greet and the other test
helpers/assertions in the same test class) and annotate each function with an
explicit return type consistent with the returned value (string, void, boolean,
Promise<...>, etc.) to satisfy the TypeScript guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc589b3c-2752-45b5-888c-ab711a5bdf51
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
controlplane/.eslintrccontrolplane/eslint-local-rules.cjscontrolplane/package.jsoncontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/SchemaCheckRepository.tscontrolplane/src/core/repositories/analytics/AnalyticsRequestViewRepository.tscontrolplane/src/core/services/BillingService.tscontrolplane/src/core/services/SchemaGraphPruner.tscontrolplane/src/core/services/SchemaLinter.tscontrolplane/src/core/tracing.tscontrolplane/src/core/webhooks/OrganizationWebhookService.tscontrolplane/test/tracing.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- controlplane/package.json
- controlplane/src/core/repositories/SchemaCheckRepository.ts
- controlplane/src/core/webhooks/OrganizationWebhookService.ts
- controlplane/src/core/services/SchemaLinter.ts
- controlplane/src/core/services/BillingService.ts
- controlplane/src/core/repositories/analytics/AnalyticsRequestViewRepository.ts
- controlplane/src/core/repositories/FederatedGraphRepository.ts
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.
Summary
@tracedclass decorator that wraps all repository and service methods withSentry.startSpan()for automatic span creationTest plan
pnpm sentry:spotlightandpnpm devwithSENTRY_ENABLED=trueand a valid DSNrequest-handler.fastify/v1/auth/session) still show spans correctlyResolves ENG-9385
Summary by CodeRabbit
New Features
Chores
Tests