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.

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

[#172201213] refactor application insights tracking #44

Merged
merged 2 commits into from
Apr 11, 2020
Merged

[#172201213] refactor application insights tracking #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac53d90
[#172201213] refactor application insights tracking
gunzip Apr 11, 2020
c2d6a0a
fix tests
gunzip Apr 11, 2020
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
20 changes: 4 additions & 16 deletions CreateMessage/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
NewMessageWithoutContent
} from "io-functions-commons/dist/src/models/message";
import { ServiceModel } from "io-functions-commons/dist/src/models/service";
import { CustomTelemetryClientFactory } from "io-functions-commons/dist/src/utils/application_insights";
import {
AzureApiAuthMiddleware,
IAzureApiAuthorization,
Expand Down Expand Up @@ -58,6 +57,7 @@ import {
ulidGenerator
} from "io-functions-commons/dist/src/utils/strings";

import { initAppInsights } from "io-functions-commons/dist/src/utils/application_insights";
import { readableReport } from "italia-ts-commons/lib/reporters";
import {
IResponseErrorForbiddenNotAuthorized,
Expand Down Expand Up @@ -337,7 +337,7 @@ const redirectToNewMessage = (
* Returns a type safe CreateMessage handler.
*/
export function CreateMessageHandler(
getCustomTelemetryClient: CustomTelemetryClientFactory,
telemetryClient: ReturnType<typeof initAppInsights>,
messageModel: MessageModel,
generateObjectId: ObjectIdGenerator
): ICreateMessageHandler {
Expand Down Expand Up @@ -376,18 +376,6 @@ export function CreateMessageHandler(
// insights
const messageId = generateObjectId();

// configure a telemetry client for application insights
const telemetryClient = getCustomTelemetryClient(
{
// each tracked event is associated to the messageId
operationId: messageId,
serviceId
},
{
messageId
}
);

// helper function used to track the message creation event in application
// insights
const trackResponse = (
Expand Down Expand Up @@ -484,12 +472,12 @@ export function CreateMessageHandler(
* Wraps a CreateMessage handler inside an Express request handler.
*/
export function CreateMessage(
getCustomTelemetryClient: CustomTelemetryClientFactory,
telemetryClient: ReturnType<typeof initAppInsights>,
serviceModel: ServiceModel,
messageModel: MessageModel
): express.RequestHandler {
const handler = CreateMessageHandler(
getCustomTelemetryClient,
telemetryClient,
messageModel,
ulidGenerator
);
Expand Down
22 changes: 10 additions & 12 deletions CreateMessage/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import {
SERVICE_COLLECTION_NAME,
ServiceModel
} from "io-functions-commons/dist/src/models/service";
import { TelemetryClient } from "io-functions-commons/dist/src/utils/application_insights";
import { wrapCustomTelemetryClient } from "io-functions-commons/dist/src/utils/application_insights";
import * as documentDbUtils from "io-functions-commons/dist/src/utils/documentdb";
import { getRequiredStringEnv } from "io-functions-commons/dist/src/utils/env";
import { secureExpressApp } from "io-functions-commons/dist/src/utils/express";
Expand All @@ -24,16 +22,12 @@ import { setAppContext } from "io-functions-commons/dist/src/utils/middlewares/c

import createAzureFunctionHandler from "io-functions-express/dist/src/createAzureFunctionsHandler";

import {
initAppInsights,
withAppInsightsContext
} from "io-functions-commons/dist/src/utils/application_insights";
import { CreateMessage } from "./handler";

// Whether we're in a production environment
const isProduction = process.env.NODE_ENV === "production";

const getCustomTelemetryClient = wrapCustomTelemetryClient(
isProduction,
new TelemetryClient()
);

// Setup Express
const app = express();
secureExpressApp(app);
Expand Down Expand Up @@ -68,9 +62,13 @@ const messageModel = new MessageModel(

const serviceModel = new ServiceModel(documentClient, servicesCollectionUrl);

const telemetryClient = initAppInsights(
getRequiredStringEnv("APPINSIGHTS_INSTRUMENTATIONKEY")
);

app.post(
"/api/v1/messages/:fiscalcode?",
CreateMessage(getCustomTelemetryClient, serviceModel, messageModel)
CreateMessage(telemetryClient, serviceModel, messageModel)
);

const azureFunctionHandler = createAzureFunctionHandler(app);
Expand All @@ -86,7 +84,7 @@ winston.add(contextTransport);
function httpStart(context: Context): void {
logger = context.log;
setAppContext(app, context);
azureFunctionHandler(context);
withAppInsightsContext(context, () => azureFunctionHandler(context));
}

export default httpStart;
68 changes: 0 additions & 68 deletions EmailNotificationActivity/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ import {
} from "io-functions-commons/dist/src/models/notification";
import { NotificationEvent } from "io-functions-commons/dist/src/models/notification_event";

import {
diffInMilliseconds,
TelemetryClient,
wrapCustomTelemetryClient
} from "io-functions-commons/dist/src/utils/application_insights";

import { generateDocumentHtml, sendMail } from "./utils";

export interface INotificationDefaults {
Expand Down Expand Up @@ -51,14 +45,6 @@ export type EmailNotificationActivityResult = t.TypeOf<
typeof EmailNotificationActivityResult
>;

// Whether we're in a production environment
const isProduction = process.env.NODE_ENV === "production";

const getCustomTelemetryClient = wrapCustomTelemetryClient(
isProduction,
new TelemetryClient()
);

/**
* Returns a function for handling EmailNotificationActivity
*/
Expand Down Expand Up @@ -93,22 +79,6 @@ export const getEmailNotificationActivityHandler = (

const logPrefix = `EmailNotificationActivity|MESSAGE_ID=${message.id}|RECIPIENT=${message.fiscalCode}|NOTIFICATION_ID=${notificationId}`;

const serviceId = message.senderServiceId;

const eventName = "handler.notification.email";

const appInsightsClient = getCustomTelemetryClient(
{
operationId: notificationId,
operationParentId: message.id,
serviceId: NonEmptyString.is(serviceId) ? serviceId : undefined
},
{
messageId: message.id,
notificationId
}
);

// Check whether the message is expired
const errorOrActiveMessage = ActiveMessage.decode(message);

Expand Down Expand Up @@ -173,8 +143,6 @@ export const getEmailNotificationActivityHandler = (
notificationDefaultParams.HTML_TO_TEXT_OPTIONS
);

const startSendMailCallTime = process.hrtime();

// trigger email delivery
// see https://nodemailer.com/message/
const sendResult = await sendMail(lMailerTransporter, {
Expand All @@ -193,49 +161,13 @@ export const getEmailNotificationActivityHandler = (
// disableUrlAccess: true,
});

const sendMailCallDurationMs = diffInMilliseconds(startSendMailCallTime);

const eventContent = {
dependencyTypeName: "HTTP",
duration: sendMailCallDurationMs,
name: "notification.email.delivery",
properties: {
addressSource: emailNotification.addressSource,
transport: lMailerTransporter.transporter.name
}
};

if (sendResult.isLeft()) {
const error = sendResult.value;
// track the event of failed delivery
appInsightsClient.trackDependency({
...eventContent,
data: error.message,
resultCode: error.name,
success: false
});
context.log.error(`${logPrefix}|ERROR=${error.message}`);
throw new Error("Error while sending email");
}

// track the event of successful delivery
appInsightsClient.trackDependency({
...eventContent,
data: "OK",
resultCode: 200,
success: true
});

appInsightsClient.trackEvent({
measurements: {
elapsed: Date.now() - notificationEvent.message.createdAt.getTime()
},
name: eventName,
properties: {
success: "true"
}
});

context.log.verbose(`${logPrefix}|RESULT=SUCCESS`);

// TODO: handling bounces and delivery updates
Expand Down
36 changes: 4 additions & 32 deletions WebhookNotificationActivity/__tests__/handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ const mockAppinsights = {
trackEvent: jest.fn()
};

const getAppinsightsMock = () => mockAppinsights;

const aFiscalCode = "FRLFRC74E04B157I" as FiscalCode;

const aMessageId = "A_MESSAGE_ID" as NonEmptyString;
Expand Down Expand Up @@ -211,9 +209,8 @@ describe("handler", () => {

await expect(
getWebhookNotificationActivityHandler(
{} as any,
getAppinsightsMock as any,
notificationModelMock as any
notificationModelMock as any,
{} as any
)(nullContext, {
notificationEvent: getMockNotificationEvent()
})
Expand All @@ -227,9 +224,8 @@ describe("handler", () => {

await expect(
getWebhookNotificationActivityHandler(
{} as any,
getAppinsightsMock as any,
notificationModelMock as any
notificationModelMock as any,
{} as any
)(nullContext, {
notificationEvent: getMockNotificationEvent()
})
Expand All @@ -243,7 +239,6 @@ describe("handler", () => {

await expect(
getWebhookNotificationActivityHandler(
getAppinsightsMock as any,
notificationModelMock as any,
{} as any
)(nullContext, {
Expand All @@ -263,21 +258,12 @@ describe("handler", () => {
.mockReturnValue(Promise.resolve(right({ status: 200 })));

const result = await getWebhookNotificationActivityHandler(
getAppinsightsMock as any,
notificationModelMock as any,
notifyCallApiMock as any
)(nullContext, {
notificationEvent: getMockNotificationEvent(aMessageContent)
});

expect(mockAppinsights.trackDependency).toHaveBeenCalledWith(
expect.objectContaining({
name: "notification.webhook.delivery",
resultCode: 200,
success: true
})
);

expect(result).toEqual({
kind: "SUCCESS",
result: "OK"
Expand All @@ -302,7 +288,6 @@ describe("handler", () => {
};

const result = await getWebhookNotificationActivityHandler(
getAppinsightsMock as any,
notificationModelMock as any,
notifyCallApiMock
)(nullContext, {
Expand All @@ -327,25 +312,13 @@ describe("handler", () => {

await expect(
getWebhookNotificationActivityHandler(
getAppinsightsMock as any,
notificationModelMock as any,
notifyCallApiMock
)(nullContext, {
notificationEvent: getMockNotificationEvent(aMessageContent)
})
).resolves.toEqual({ kind: "FAILURE", reason: "SEND_TO_WEBHOOK_FAILED" });

expect(mockAppinsights.trackDependency).toHaveBeenCalledWith(
expect.objectContaining({
name: "notification.webhook.delivery",
properties: {
error: "Permanent HTTP error calling webhook: 401"
},
resultCode: "PermanentError",
success: false
})
);

expect(notificationModelMock.update).not.toHaveBeenCalled();
});

Expand All @@ -357,7 +330,6 @@ describe("handler", () => {
const notificationEvent = getMockNotificationEvent();

const ret = await getWebhookNotificationActivityHandler(
getAppinsightsMock as any,
notificationModelMock as any,
{} as any
)(nullContext, {
Expand Down
Loading