Skip to content
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

[FIX] Allow using a custom error fingerprint in simple error logs #796

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 7 additions & 6 deletions packages/core/src/logs/DdLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ const isLogWithError = (
args: LogArguments | LogWithErrorArguments
): args is LogWithErrorArguments => {
return (
(args.length > 3 &&
(args[1] !== undefined ||
args[2] !== undefined ||
args[3] !== undefined)) ||
typeof args[1] === 'string' ||
typeof args[2] === 'string' ||
typeof args[3] === 'string' ||
Expand Down Expand Up @@ -105,6 +109,7 @@ class DdLogsWrapper implements DdLogsType {
args[6]
);
}

return this.log(args[0], validateContext(args[1]), 'error');
};

Expand Down Expand Up @@ -180,7 +185,7 @@ class DdLogsWrapper implements DdLogsType {
stacktrace: string | undefined,
context: object,
status: 'debug' | 'info' | 'warn' | 'error',
fingerprint?: string,
fingerprint: string = '',
source?: ErrorSource
): Promise<void> => {
const rawLogEvent: RawLogWithError = {
Expand All @@ -190,7 +195,7 @@ class DdLogsWrapper implements DdLogsType {
stacktrace,
context,
status,
fingerprint: fingerprint ?? '',
fingerprint,
source
};

Expand All @@ -208,10 +213,6 @@ class DdLogsWrapper implements DdLogsType {
[DdAttributes.errorSourceType]: 'react-native'
};

if (fingerprint && fingerprint !== '') {
updatedContext[DdAttributes.errorFingerprint] = fingerprint;
}

return await this.nativeLogs[`${status}WithError`](
mappedEvent.message,
(mappedEvent as NativeLogWithError).errorKind,
Expand Down
95 changes: 58 additions & 37 deletions packages/core/src/logs/__tests__/DdLogs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import { InternalLog } from '../../InternalLog';
import { SdkVerbosity } from '../../SdkVerbosity';
import type { DdNativeLogsType } from '../../nativeModulesTypes';
import { ErrorSource } from '../../rum/types';
import { GlobalState } from '../../sdk/GlobalState/GlobalState';
import { DdLogs } from '../DdLogs';
import type { LogEventMapper } from '../types';
import type { LogEvent, LogEventMapper } from '../types';

jest.mock('../../InternalLog', () => {
return {
Expand All @@ -24,7 +25,34 @@ jest.mock('../../InternalLog', () => {
};
});

const initializeSdk = async () => {
if (GlobalState.instance.isInitialized) {
return;
}

// GIVEN
const fakeAppId = '1';
const fakeClientToken = '2';
const fakeEnvName = 'env';
const configuration = new DdSdkReactNativeConfiguration(
fakeClientToken,
fakeEnvName,
fakeAppId,
false,
false,
true // Track Errors
);

NativeModules.DdSdk.initialize.mockResolvedValue(null);

// WHEN
await DdSdkReactNative.initialize(configuration);
};

describe('DdLogs', () => {
beforeAll(async () => {
await initializeSdk();
});
describe('log event mapper', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -194,35 +222,45 @@ describe('DdLogs', () => {
);
});

it('console errors can be filtered with mappers when trackErrors=true', async () => {
it('fingerprint can be injected with mappers in error logs', async () => {
// GIVEN
const fakeAppId = '1';
const fakeClientToken = '2';
const fakeEnvName = 'env';
const configuration = new DdSdkReactNativeConfiguration(
fakeClientToken,
fakeEnvName,
fakeAppId,
false,
false,
true // Track Errors
const errorFingerprint = 'my-custom-fingerprint';

// Register log event mapper to add error fingerprint
DdLogs.registerLogEventMapper((logEvent: LogEvent) => {
logEvent.fingerprint = errorFingerprint;
return logEvent;
});

console.error('test-error');

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unexpected console statement. (...read more)

Debugging with console is not considered a bad practice, but it's easy to forget about console statements and leave them in production code. There is no need to pollute production builds with debugging statements.

View in Datadog  Leave us feedback  Documentation

expect(NativeModules.DdLogs.errorWithError).toHaveBeenCalledWith(
'test-error',
'Error',
'test-error',
'',
{
'_dd.error.fingerprint': errorFingerprint,
'_dd.error.source_type': 'react-native',
'_dd.error_log.is_crash': true
}
);
});

// Register log event mapper to filter console log events
configuration.logEventMapper = logEvent => {
it('console errors can be filtered with mappers when trackErrors=true', async () => {
// GIVEN
// Log event mapper to filter console log events
DdLogs.registerLogEventMapper(logEvent => {
if (logEvent.source === ErrorSource.CONSOLE) {
return null;
}

return logEvent;
};

NativeModules.DdSdk.initialize.mockResolvedValue(null);
});

// WHEN
await DdSdkReactNative.initialize(configuration);

console.error('console-error-message');

// THEN
expect(NativeModules.DdLogs.error).not.toHaveBeenCalled();
expect(InternalLog.log).toHaveBeenCalledWith(
'error log dropped by log mapper: "console-error-message"',
Expand Down Expand Up @@ -257,24 +295,7 @@ describe('DdLogs', () => {
});

it('console errors are reported in logs when trackErrors=true', async () => {
// GIVEN
const fakeAppId = '1';
const fakeClientToken = '2';
const fakeEnvName = 'env';
const configuration = new DdSdkReactNativeConfiguration(
fakeClientToken,
fakeEnvName,
fakeAppId,
false,
false,
true // Track Errors
);

NativeModules.DdSdk.initialize.mockResolvedValue(null);

// WHEN
await DdSdkReactNative.initialize(configuration);

// trackErrors is true in initializeSdk() configuration
console.error('console-error-message');
expect(NativeModules.DdLogs.error).not.toHaveBeenCalled();
expect(InternalLog.log).toHaveBeenCalledWith(
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/logs/eventMapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* Copyright 2016-Present Datadog, Inc.
*/

import { DdAttributes } from '../rum/DdAttributes';
import type { Attributes } from '../sdk/AttributesSingleton/types';
import { EventMapper } from '../sdk/EventMappers/EventMapper';
import type { UserInfo } from '../sdk/UserInfoSingleton/types';

import { InternalLogEvent } from './types';
import type {
LogEvent,
LogEventMapper,
Expand All @@ -20,12 +22,17 @@ import type {
export const formatLogEventToNativeLog = (
logEvent: LogEvent
): NativeLog | NativeLogWithError => {
return logEvent;
return new InternalLogEvent(logEvent);
};

export const formatRawLogToNativeEvent = (
rawLog: RawLog | RawLogWithError
): NativeLog | NativeLogWithError => {
if ((rawLog as RawLogWithError).fingerprint) {
(rawLog.context as any)[

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unsafe assignment of an `any` value. (...read more)

The any type in TypeScript is dangerously broad, leading to unexpected behavior. Using any should be avoided.

View in Datadog  Leave us feedback  Documentation

DdAttributes.errorFingerprint
] = (rawLog as RawLogWithError).fingerprint;
}
return rawLog;
};

Expand All @@ -36,11 +43,11 @@ export const formatRawLogToLogEvent = (
attributes: Attributes;
}
): LogEvent => {
return {
return new InternalLogEvent({
...rawLog,
userInfo: additionalInformation.userInfo,
attributes: additionalInformation.attributes
};
});
};

export const generateEventMapper = (
Expand Down
33 changes: 33 additions & 0 deletions packages/core/src/logs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Copyright 2016-Present Datadog, Inc.
*/

import { DdAttributes } from '../rum/DdAttributes';
import type { ErrorSource } from '../rum/types';
import type { UserInfo } from '../sdk/UserInfoSingleton/types';

Expand Down Expand Up @@ -91,6 +92,38 @@ export type LogEvent = {
readonly attributes?: object;
};

export class InternalLogEvent implements LogEvent {
message: string;
context: object;
errorKind?: string | undefined;
errorMessage?: string | undefined;
stacktrace?: string | undefined;
source?: ErrorSource | undefined;
status: LogStatus;
userInfo: UserInfo;
attributes?: object | undefined;

public get fingerprint(): string | undefined {
return (this.context as any)[DdAttributes.errorFingerprint] as string;

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unsafe assignment of an `any` value. (...read more)

The any type in TypeScript is dangerously broad, leading to unexpected behavior. Using any should be avoided.

View in Datadog  Leave us feedback  Documentation

}
public set fingerprint(value: string | undefined) {
(this.context as any)[DdAttributes.errorFingerprint] = value;

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unsafe assignment of an `any` value. (...read more)

The any type in TypeScript is dangerously broad, leading to unexpected behavior. Using any should be avoided.

View in Datadog  Leave us feedback  Documentation

}

constructor(props: LogEvent) {
this.message = props.message;
this.context = props.context;
this.errorKind = props.errorKind;
this.errorMessage = props.errorMessage;
this.stacktrace = props.stacktrace;
this.fingerprint = props.fingerprint;
this.status = props.status;
this.source = props.source;
this.userInfo = props.userInfo;
this.attributes = props.attributes;
}
}

export type LogEventMapper = (logEvent: LogEvent) => LogEvent | null;

export type LogArguments = [message: string, context?: object];
Expand Down