Skip to content

chore: Add deviceType and appMode to new relic telemetry data #33735

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.

Sign up for GitHub

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

Merged
rajatagrawal merged 7 commits into from
May 31, 2024
Merged

chore: Add deviceType and appMode to new relic telemetry data #33735

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5364e4d
chore: Add deviceType and appMode to new relic telemetry data
May 27, 2024
50a4c93
Fix jest test case failures
May 27, 2024
094ab42
Merge remote-tracking branch 'origin' into chore/add-attributes-telem…
May 28, 2024
576c458
Move appmode to common attributes
May 29, 2024
67184f0
Add window pathname in common telemetry attribute
May 29, 2024
b305590
Merge branch 'release' into chore/add-attributes-telemetry
May 29, 2024
309d8ae
Merge remote-tracking branch 'origin' into chore/add-attributes-telem…
May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 45 additions & 11 deletions app/client/src/UITelemetry/generateTraces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,54 @@
import type { Span, Attributes, TimeInput } from "@opentelemetry/api";
import type {
Span,
Attributes,
TimeInput,
SpanOptions,
} from "@opentelemetry/api";
import { SpanKind } from "@opentelemetry/api";
import { context } from "@opentelemetry/api";
import { trace } from "@opentelemetry/api";
import { deviceType } from "react-device-detect";
Comment thread
dvj1988 marked this conversation as resolved.

import { APP_MODE } from "entities/App";
import { matchBuilderPath, matchViewerPath } from "constants/routes";

const GENERATOR_TRACE = "generator-tracer";

const getCommonTelemetryAttributes = () => {
const pathname = window.location.pathname;
const isEditorUrl = matchBuilderPath(pathname);
Comment thread
rajatagrawal marked this conversation as resolved.
const isViewerUrl = matchViewerPath(pathname);

const appMode = isEditorUrl
? APP_MODE.EDIT
: isViewerUrl
? APP_MODE.PUBLISHED
: "";

return {
appMode,
deviceType,

@rajatagrawal rajatagrawal May 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please add appMode also as common telemetry attribute ?

@dvj1988 dvj1988 May 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

@dvj1988 dvj1988 May 28, 2024 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajatagrawal The test cases were failing because the worker was trying to access the window.location object which is not available in the worker's scope.
This is happening because we are creating the root span inside the request method of the worker service and this makes it impossible to access the common attributes unless they are passed as an additional argument to the request method. I am reverting the code to the previous state for now.

};
Comment thread
dvj1988 marked this conversation as resolved.
Comment thread
dvj1988 marked this conversation as resolved.
};

export function startRootSpan(
spanName: string,
spanAttributes?: Attributes,
spanAttributes: Attributes = {},
startTime?: TimeInput,
) {
const tracer = trace.getTracer(GENERATOR_TRACE);
if (!spanName) {
return;
}
const attributes = spanAttributes ?? { attributes: spanAttributes };
const startTimeAttr = startTime ? { startTime } : {};
const commonAttributes = getCommonTelemetryAttributes();

return tracer?.startSpan(spanName, {
kind: SpanKind.CLIENT,
...attributes,
...startTimeAttr,
attributes: {
...commonAttributes,
...spanAttributes,
},
startTime,
Comment on lines 36 to 51

@coderabbitai coderabbitai Bot May 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper error handling in startRootSpan.

The function startRootSpan uses optional chaining (tracer?.startSpan) which could lead to silent failures if tracer is undefined. It's better to handle this explicitly:

  const tracer = trace.getTracer(GENERATOR_TRACE);
+ if (!tracer) {
+   throw new Error('Tracer not found for GENERATOR_TRACE');
+ }
  if (!spanName) {
    return;
  }
  ...
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
spanAttributes: Attributes = {},
startTime?: TimeInput,
) {
const tracer = trace.getTracer(GENERATOR_TRACE);
if (!spanName) {
return;
}
const attributes = spanAttributes ?? { attributes: spanAttributes };
const startTimeAttr = startTime ? { startTime } : {};
const commonAttributes = getCommonTelemetryAttributes();
return tracer?.startSpan(spanName, {
kind: SpanKind.CLIENT,
...attributes,
...startTimeAttr,
attributes: {
...commonAttributes,
...spanAttributes,
},
startTime,
spanAttributes: Attributes = {},
startTime?: TimeInput,
) {
const tracer = trace.getTracer(GENERATOR_TRACE);
if (!tracer) {
throw new Error('Tracer not found for GENERATOR_TRACE');
}
if (!spanName) {
return;
}
const commonAttributes = getCommonTelemetryAttributes();
return tracer.startSpan(spanName, {
kind: SpanKind.CLIENT,
attributes: {
...commonAttributes,
...spanAttributes,
},
startTime,

Comment thread
dvj1988 marked this conversation as resolved.
});
}
export const generateContext = (span: Span) => {
Expand All @@ -27,7 +57,7 @@ export const generateContext = (span: Span) => {
export function startNestedSpan(
spanName: string,
parentSpan?: Span,
spanAttributes?: Attributes,
spanAttributes: Attributes = {},
startTime?: TimeInput,
) {
if (!spanName || !parentSpan) {
Expand All @@ -38,13 +68,17 @@ export function startNestedSpan(
const parentContext = generateContext(parentSpan);

const generatorTrace = trace.getTracer(GENERATOR_TRACE);
const commonAttributes = getCommonTelemetryAttributes();

const attributes = {
const spanOptions: SpanOptions = {
kind: SpanKind.CLIENT,
...(startTime ? { startTime } : {}),
...(spanAttributes ? { attributes: spanAttributes } : {}),
attributes: {
...commonAttributes,
...spanAttributes,
},
startTime,
};
return generatorTrace.startSpan(spanName, attributes, parentContext);
return generatorTrace.startSpan(spanName, spanOptions, parentContext);
}

export function endSpan(span?: Span) {
Expand Down