-
Notifications
You must be signed in to change notification settings - Fork 478
extensions observability #422
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
63796b8
to
9d2bac4
Compare
09d3205
to
3550452
Compare
9d2bac4
to
88ba007
Compare
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.
Tested on main PR 💯
Code changes LGTM
3550452
to
3c106dd
Compare
src/vs/workbench/services/telemetry/browser/telemetryService.ts
Outdated
Show resolved
Hide resolved
88ba007
to
cd660e9
Compare
cd660e9
to
5f2621e
Compare
As I was on holiday on friday I didn't have time to do a proper review, will add comments so maybe they are addressed in the future |
|
||
export class GitpodInsightsAppender implements ITelemetryAppender { | ||
private _baseProperties: { appName: string; uiKind: 'web'; version: string }; | ||
private readonly _baseProperties: { appName: string; uiKind: 'web'; version: string }; | ||
private readonly devMode = this.productService.nameShort.endsWith(' Dev'); |
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.
This doesn't look nice, should be moved to workbench.ts and if true then set gitpodPreview
to undefined
and use that as a check here
window.postMessage({ type: 'vscode_telemetry', event: trackMessage.event, properties: trackMessage.properties }, '*'); | ||
} | ||
|
||
private async sendMetrics(eventName: string, data: any): Promise<void> { |
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 browser gitpodInsightsAppender.ts is just and edge for the reconnection events when we lose connection to the remote server, all other events will be send to the remote so not sure if we need the sendMetrics
logic here
|
||
export function mapTelemetryData(kind: SenderKind, eventName: string, data: any): RemoteTrackMessage | undefined { | ||
if (kind === SenderKind.Node) { |
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.
Why not keep using the enum?
export class GitpodInsightsAppender implements ITelemetryAppender { | ||
|
||
private _asyncAIClient: Promise<GitpodConnection> | null; | ||
private _defaultData: { [key: string]: any } = Object.create(null); | ||
private _baseProperties: { appName: string; uiKind: 'web'; version: string }; | ||
private readonly supervisor = new SupervisorConnection(); | ||
private readonly devMode = this.productName.endsWith(' Dev'); |
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.
Same as before, move to workbench.ts
Supporting for gitpod-io/gitpod#12539. See this PR for explanation and how to test.