-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: add custom OpenTelemetry collector endpoints support #7023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add OpenTelemetryClient class to packages/telemetry for sending traces to custom OTEL endpoints - Add OTEL configuration fields (otelEnabled, otelEndpoints) to global settings schema - Create UI components in settings view for managing OTEL endpoints with add/remove/edit capabilities - Integrate OTEL client initialization in extension startup - Add message handlers for OTEL settings updates between webview and extension - Include comprehensive test coverage for OpenTelemetryClient - Support multiple endpoints with custom headers and enable/disable toggles per endpoint Implements #7020
setCachedStateField: SetCachedStateField<"otelEnabled" | "otelEndpoints"> | ||
} | ||
|
||
export const OpenTelemetrySettings = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the i18n translation function (e.g. t('...')) for all user‐visible strings like 'OpenTelemetry', 'Add Endpoint', and 'Collector Endpoints' to ensure consistency with the rest of the app.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
|
||
// Create resource with service information | ||
const version = | ||
vscode?.extensions?.getExtension?.("rooveterinaryinc.roo-cline")?.packageJSON?.version || "unknown" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extension identifier "rooveterinaryinc.roo-cline" might be a typographical error. Should it be "rooveterinaryinc.roo-code" or another identifier? Please confirm.
vscode?.extensions?.getExtension?.("rooveterinaryinc.roo-cline")?.packageJSON?.version || "unknown" | |
vscode?.extensions?.getExtension?.("rooveterinaryinc.roo-code")?.packageJSON?.version || "unknown" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code because apparently I trust no one, not even myself.
|
||
// Create resource with service information | ||
const version = | ||
vscode?.extensions?.getExtension?.("rooveterinaryinc.roo-cline")?.packageJSON?.version || "unknown" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this extension ID intentional? I noticed it's hardcoded as 'rooveterinaryinc.roo-cline' - should this perhaps be updated to match the actual extension ID or made configurable?
} | ||
} | ||
|
||
export interface OtelEndpoint { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding JSDoc comments for the OtelEndpoint interface properties to clarify what each field represents:
|
||
// Add exporters for each endpoint | ||
for (const endpoint of this.endpoints) { | ||
const exporter = new OTLPTraceExporter({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be beneficial to add retry logic here for transient network failures? Other telemetry implementations in the codebase seem to have retry mechanisms.
URL | ||
</label> | ||
<VSCodeTextField | ||
value={endpoint.url} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding basic URL validation here to prevent configuration errors. Something like:
Then you could show a validation error if the URL is invalid.
)} | ||
|
||
<div className="text-vscode-descriptionForeground text-sm mt-2"> | ||
<strong>Note:</strong> Changes to endpoints require restarting VS Code to take effect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The restart requirement note is important but might be missed at the bottom. Consider making it more prominent, perhaps showing a notification when settings are saved, or moving this warning to the top of the section?
Author interested in implementing this |
Implements #7020 - Adds the ability to configure custom OpenTelemetry (OTEL) collector endpoints for telemetry traces.
Summary
This PR adds support for configuring custom OpenTelemetry collector endpoints, allowing power users to send telemetry traces to their own collectors for visibility into model/inference activity.
Changes
Core Implementation
packages/telemetry/src/OpenTelemetryClient.ts
that:Configuration
otelEnabled
andotelEndpoints
fields to the global settings schemaUser Interface
OpenTelemetrySettings
component in the settings viewIntegration
Testing
Testing Instructions
https://otel-collector.example.com/v1/traces
)Notes
Closes #7020
Important
Adds support for custom OpenTelemetry collector endpoints, allowing users to configure and send telemetry traces to their own endpoints.
OpenTelemetryClient
inOpenTelemetryClient.ts
to handle telemetry with custom OTEL endpoints.otelEnabled
andotelEndpoints
to global settings schema inglobal-settings.ts
.OpenTelemetrySettings
component inOpenTelemetrySettings.tsx
for configuring OTEL settings.OpenTelemetryClient
inextension.ts
if enabled in settings.webviewMessageHandler.ts
to handle OTEL settings updates.OpenTelemetryClient.test.ts
for initialization, event capture, and error handling.package.json
to include OpenTelemetry dependencies.This description was created by
for bb00d6c. You can customize this summary. It will automatically update as commits are pushed.