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

Add eventCode to event input #943

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions packages/integration-sdk-core/src/types/logger.ts
Original file line number Diff line number Diff line change
@@ -40,6 +40,10 @@ export type IntegrationEvent = {
export type PublishEventInput = {
name: string;
description: string;
/**
* Allows the event to be identified via a code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add examples to the docstrings?

*/
eventCode?: string;
level?: PublishEventLevel;
};

Original file line number Diff line number Diff line change
@@ -368,6 +368,7 @@ describe('step event publishing', () => {
expect(onEmitEvent).toHaveBeenNthCalledWith(8, {
name: 'step_failure',
level: PublishEventLevel.Error,
eventCode: expect.toBeString(),
description: expect.stringMatching(
new RegExp(
`Step "Mochi" failed to complete due to error. \\(errorCode="${error.code}", reason="ripperoni"\\)$`,
@@ -423,6 +424,7 @@ describe('provider auth error details', () => {
expect(onEmitEvent).toHaveBeenCalledWith({
name: 'step_failure',
level: PublishEventLevel.Error,
eventCode: expect.toBeString(),
description: expect.stringMatching(
new RegExp(
'^Step "Mochi" failed to complete due to error.' +
@@ -439,6 +441,7 @@ describe('provider auth error details', () => {
expect(onEmitEvent).toHaveBeenCalledWith({
name: 'validation_failure',
level: PublishEventLevel.Error,
eventCode: expect.toBeString(),
description: expect.stringMatching(
new RegExp(
'^Error occurred while validating integration configuration.' +
@@ -503,6 +506,7 @@ describe('validation failure logging', () => {
expect(onEmitEvent).toHaveBeenNthCalledWith(1, {
name: 'validation_failure',
level: PublishEventLevel.Error,
eventCode: expect.toBeString(),
description: expect.stringMatching(expectedDescriptionRegex),
});

@@ -535,6 +539,7 @@ describe('validation failure logging', () => {
expect(onEmitEvent).toHaveBeenNthCalledWith(1, {
name: 'validation_failure',
level: PublishEventLevel.Error,
eventCode: expect.toBeString(),
description: expect.stringMatching(expectedDescriptionRegex),
});

@@ -731,13 +736,15 @@ describe('#publishEvent', () => {
logger.publishWarnEvent({
name: IntegrationWarnEventName[key],
description: 'the description',
eventCode: 'AWS-PER-ECS',
});

expect(onEmitEvent).toHaveBeenCalledTimes(1);
expect(onEmitEvent).toHaveBeenCalledWith({
name: IntegrationWarnEventName[key],
level: PublishEventLevel.Warn,
description: 'the description',
eventCode: 'AWS-PER-ECS',
});
},
);
@@ -756,6 +763,7 @@ describe('#publishEvent', () => {
logger.publishErrorEvent({
name: IntegrationErrorEventName[key],
description: 'the description',
eventCode: 'AWS-PER-ECS',
});

expect(onEmitEvent).toHaveBeenCalledTimes(1);
@@ -764,6 +772,7 @@ describe('#publishEvent', () => {
name: IntegrationErrorEventName[key],
level: PublishEventLevel.Error,
description: 'the description',
eventCode: 'AWS-PER-ECS',
});
},
);
27 changes: 21 additions & 6 deletions packages/integration-sdk-runtime/src/logger/index.ts
Original file line number Diff line number Diff line change
@@ -194,7 +194,7 @@ export class IntegrationLogger
}

/**
* Answers `true` when the err has been reported to the logger instance
* Answers `true` when the error has been reported to the logger instance
* through these functions:
*
* * warn(err, ...)
@@ -366,11 +366,17 @@ export class IntegrationLogger

stepFailure(step: StepMetadata, err: Error) {
const eventName = 'step_failure';
const { errorId, description } = createErrorEventDescription(
const { errorId, errorCode, description } = createErrorEventDescription(
err,
`Step "${step.name}" failed to complete due to error.`,
);
this.handleFailure({ eventName, errorId, err, description });
this.handleFailure({
eventName,
errorId,
eventCode: errorCode,
err,
description,
});
}

synchronizationUploadStart(job: SynchronizationJob) {
@@ -399,20 +405,27 @@ export class IntegrationLogger

validationFailure(err: Error) {
const eventName = 'validation_failure';
const { errorId, description } = createErrorEventDescription(
const { errorId, errorCode, description } = createErrorEventDescription(
Copy link
Contributor Author

@VDubber VDubber Aug 16, 2023

Choose a reason for hiding this comment

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

All we get from the graph projects is an error during the validationFailure phase. We could enhance this but I don't know if it is necessary. Since integrationType + validationFailure is rather unique.

err,
`Error occurred while validating integration configuration.`,
);
this.handleFailure({ eventName, errorId, err, description });
this.handleFailure({
eventName,
errorId,
eventCode: errorCode,
err,
description,
});
}

private handleFailure(options: {
eventName: 'validation_failure' | 'step_failure';
errorId: string;
eventCode: string;
err: Error;
description: string;
}) {
const { eventName, errorId, err, description } = options;
const { eventName, errorId, eventCode, err, description } = options;

// If there is a `code` property on the `Error`, we should include this
// in our log. This is helpful for when we receive an HTTP response error
@@ -429,6 +442,7 @@ export class IntegrationLogger
this.publishEvent({
name: eventName,
description,
eventCode,
level: PublishEventLevel.Error,
});
}
@@ -529,6 +543,7 @@ export function createErrorEventDescription(

return {
errorId,
errorCode,
description: `${message} (${errorDetails})`,
};
}