Skip to content

Commit

Permalink
Refactor logger (#1635)
Browse files Browse the repository at this point in the history
* Refactor how we use loggers

* cleanup leftovers

* Improve tests
  • Loading branch information
krzysztofzuraw authored Oct 25, 2024
1 parent 6d528dc commit d088ef3
Show file tree
Hide file tree
Showing 18 changed files with 156 additions and 92 deletions.
10 changes: 10 additions & 0 deletions .changeset/healthy-pianos-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"products-feed": patch
"klaviyo": patch
"app-avatax": patch
"cms-v2": patch
"search": patch
"smtp": patch
---

Use new way of creating logger from `@saleor/apps-logger`
5 changes: 5 additions & 0 deletions .changeset/violet-candles-hide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@saleor/apps-logger": minor
---

Renamed exported `logger` to `rootLogger` so we avoid collision of names when using `logger` in monorepo. Also `createLogger` function has been removed in favour of app defining it in their codebase.
16 changes: 11 additions & 5 deletions apps/avatax/scripts/migration-logger.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
// eslint-disable-next-line no-restricted-imports
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";
import { attachLoggerOtelTransport } from "@saleor/apps-logger/node";

import packageJson from "../package.json";
import { loggerContext } from "../src/logger-context";

logger.settings.maskValuesOfKeys = ["username", "password", "token"];
rootLogger.settings.maskValuesOfKeys = ["username", "password", "token"];

if (process.env.ENABLE_MIGRATION_CONSOLE_LOGGER === "true") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

attachLoggerOtelTransport(logger, packageJson.version, loggerContext);
attachLoggerOtelTransport(rootLogger, packageJson.version, loggerContext);

export { createLogger, logger };
export const createMigrationScriptLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
4 changes: 2 additions & 2 deletions apps/avatax/scripts/run-webhooks-migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { WebhookMigrationRunner } from "@saleor/webhook-utils";
import { createInstrumentedGraphqlClient } from "../src/lib/create-instrumented-graphql-client";
import { loggerContext } from "../src/logger-context";
import { appWebhooks } from "../webhooks";
import { createLogger } from "./migration-logger";
import { createMigrationScriptLogger } from "./migration-logger";

const requiredEnvs = ["REST_APL_TOKEN", "REST_APL_ENDPOINT"];

Expand All @@ -17,7 +17,7 @@ if (process.env.OTEL_ENABLED === "true" && process.env.OTEL_SERVICE_NAME) {
otelSdk.start();
}

const logger = createLogger("RunWebhooksMigration");
const logger = createMigrationScriptLogger("RunWebhooksMigration");

const runMigrations = async () => {
if (!requiredEnvs.every((env) => process.env[env])) {
Expand Down
4 changes: 2 additions & 2 deletions apps/avatax/src/lib/app-configuration-logger.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { ObservabilityAttributes } from "@saleor/apps-otel/src/lib/observability-attributes";

import { logger } from "../logger";
import { createLogger } from "../logger";
import { AppConfig } from "./app-config";

export class AppConfigurationLogger {
constructor(private injectedLogger: Pick<typeof logger, "info" | "warn">) {}
constructor(private injectedLogger: Pick<ReturnType<typeof createLogger>, "info" | "warn">) {}

logConfiguration(configuration: AppConfig, channelSlug: string) {
const config = configuration.getConfigForChannelSlug(channelSlug);
Expand Down
5 changes: 3 additions & 2 deletions apps/avatax/src/lib/error-utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { TRPCClientError } from "@trpc/client";
import { GraphQLError } from "graphql";

import { createLogger } from "@/logger";

import { BaseError } from "../error";
import { logger } from "../logger";
import { CalculateTaxesPayload } from "../modules/webhooks/payloads/calculate-taxes-payload";
import { OrderCancelledPayload } from "../modules/webhooks/payloads/order-cancelled-payload";
import { OrderConfirmedPayload } from "../modules/webhooks/payloads/order-confirmed-payload";
Expand All @@ -21,7 +22,7 @@ export class SubscriptionPayloadErrorChecker {
private handledErrorPath = ["event", "taxBase", "sourceObject", "user"];

constructor(
private injectedLogger: Pick<typeof logger, "error" | "info">,
private injectedLogger: Pick<ReturnType<typeof createLogger>, "error" | "info">,
private injectedErrorCapture: (
exception: InstanceType<typeof SubscriptionPayloadErrorChecker.SubscriptionPayloadError>,
) => void,
Expand Down
18 changes: 12 additions & 6 deletions apps/avatax/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";

import packageJson from "../package.json";

logger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];
rootLogger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];

if (process.env.NODE_ENV !== "production") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

if (typeof window === "undefined") {
Expand All @@ -15,15 +15,21 @@ if (typeof window === "undefined") {
attachLoggerVercelTransport,
} = require("@saleor/apps-logger/node");

attachLoggerSentryTransport(logger);
attachLoggerSentryTransport(rootLogger);

if (process.env.NODE_ENV === "production") {
attachLoggerVercelTransport(
logger,
rootLogger,
packageJson.version,
require("./logger-context").loggerContext,
);
}
}

export { createLogger, logger };
export const createLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
19 changes: 13 additions & 6 deletions apps/cms-v2/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";

import packageJson from "../package.json";

logger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];
rootLogger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];

if (process.env.NODE_ENV !== "production") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

if (typeof window === "undefined") {
import("@saleor/apps-logger/node").then(
async ({ attachLoggerOtelTransport, attachLoggerSentryTransport, LoggerContext }) => {
const loggerContext = await import("./logger-context").then((m) => m.loggerContext);

attachLoggerSentryTransport(logger);
attachLoggerOtelTransport(logger, packageJson.version, loggerContext);
attachLoggerSentryTransport(rootLogger);
attachLoggerOtelTransport(rootLogger, packageJson.version, loggerContext);
},
);
}

export { createLogger, logger };
export const createLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import * as Sentry from "@sentry/nextjs";
import { ClientAPI, createClient, Environment } from "contentful-management";
import { WebhookProductVariantFragment } from "../../../../generated/graphql";

import { createLogger } from "@/logger";
import { ContentfulProviderConfig } from "@/modules/configuration";

import * as Sentry from "@sentry/nextjs";
import { createLogger, logger } from "@/logger";
import { WebhookProductVariantFragment } from "../../../../generated/graphql";

type ConstructorOptions = {
space: string;
Expand All @@ -30,7 +31,7 @@ export class ContentfulClient {
private client: ContentfulApiClientChunk;
private space: string;

private logger: typeof logger;
private logger: ReturnType<typeof createLogger>;

constructor(opts: ConstructorOptions, clientFactory: SdkClientFactory = defaultSdkClientFactory) {
this.space = opts.space;
Expand Down Expand Up @@ -276,7 +277,7 @@ export class ContentfulClient {
return this.uploadProductVariant({ configuration, variant });
}
} catch (err) {
logger.error("Error during the upsert", { error: err });
this.logger.error("Error during the upsert", { error: err });
Sentry.captureException(err);

throw err;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { createLogger } from "@/logger";
import { PayloadCmsProviderConfig } from "@/modules/configuration/schemas/payloadcms-provider.schema";

import { BulkImportProductFragment } from "../../../../generated/graphql";
import { BulkSyncProcessor, BulkSyncProcessorHooks } from "../../bulk-sync/bulk-sync-processor";

import { PayloadCmsProviderConfig } from "@/modules/configuration/schemas/payloadcms-provider.schema";
import { PayloadCMSClient } from "./payloadcms-client";
import { createLogger, logger } from "@/logger";

// todo CORS or proxy
export class PayloadCmsBulkSyncProcessor implements BulkSyncProcessor {
Expand Down
10 changes: 6 additions & 4 deletions apps/cms-v2/src/modules/trpc/protected-client-procedure.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { ProtectedHandlerError } from "@saleor/app-sdk/handlers/next";
import { verifyJWT } from "@saleor/app-sdk/verify-jwt";
import { middleware, procedure } from "./trpc-server";
import { REQUIRED_SALEOR_PERMISSIONS } from "@saleor/apps-shared";
import { TRPCError } from "@trpc/server";
import { ProtectedHandlerError } from "@saleor/app-sdk/handlers/next";

import { createLogger } from "@/logger";

import { saleorApp } from "../../saleor-app";
import { createLogger, logger as appLogger } from "@/logger";
import { createInstrumentedGraphqlClient } from "./create-instrumented-graphql-client";
import { REQUIRED_SALEOR_PERMISSIONS } from "@saleor/apps-shared";
import { middleware, procedure } from "./trpc-server";

const logger = createLogger("protectedClientProcedure");

Expand Down
19 changes: 13 additions & 6 deletions apps/klaviyo/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";

import packageJson from "../package.json";

logger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];
rootLogger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];

if (process.env.NODE_ENV !== "production") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

if (typeof window === "undefined") {
import("@saleor/apps-logger/node").then(
async ({ attachLoggerOtelTransport, attachLoggerSentryTransport }) => {
const loggerContext = await import("./logger-context").then((m) => m.loggerContext);

attachLoggerSentryTransport(logger);
attachLoggerOtelTransport(logger, packageJson.version, loggerContext);
attachLoggerSentryTransport(rootLogger);
attachLoggerOtelTransport(rootLogger, packageJson.version, loggerContext);
},
);
}

export { createLogger, logger };
export const createLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
21 changes: 14 additions & 7 deletions apps/products-feed/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";

import packageJson from "../package.json";

logger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];
rootLogger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];

if (process.env.NODE_ENV !== "production") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

if (typeof window === "undefined") {
import("@saleor/apps-logger/node").then(
async ({ attachLoggerOtelTransport, attachLoggerSentryTransport, LoggerContext }) => {
async ({ attachLoggerOtelTransport, attachLoggerSentryTransport }) => {
const loggerContext = await import("./logger-context").then((m) => m.loggerContext);

attachLoggerSentryTransport(logger);
attachLoggerOtelTransport(logger, packageJson.version, loggerContext);
attachLoggerSentryTransport(rootLogger);
attachLoggerOtelTransport(rootLogger, packageJson.version, loggerContext);
},
);
}

export { createLogger, logger };
export const createLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
19 changes: 13 additions & 6 deletions apps/search/src/lib/logger.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
// eslint-disable-next-line no-restricted-imports
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";

import packageJson from "../../package.json";

logger.settings.maskValuesOfKeys = ["token", "secretKey"];
rootLogger.settings.maskValuesOfKeys = ["token", "secretKey"];

if (process.env.NODE_ENV !== "production") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

if (typeof window === "undefined") {
import("@saleor/apps-logger/node").then(
({ attachLoggerOtelTransport, attachLoggerSentryTransport }) => {
attachLoggerSentryTransport(logger);
attachLoggerOtelTransport(logger, packageJson.version);
attachLoggerSentryTransport(rootLogger);
attachLoggerOtelTransport(rootLogger, packageJson.version);
},
);
}

export { createLogger, logger };
export const createLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
19 changes: 13 additions & 6 deletions apps/smtp/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
import { attachLoggerConsoleTransport, createLogger, logger } from "@saleor/apps-logger";
import { attachLoggerConsoleTransport, rootLogger } from "@saleor/apps-logger";

import packageJson from "../package.json";

logger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];
rootLogger.settings.maskValuesOfKeys = ["metadata", "username", "password", "apiKey"];

if (process.env.NODE_ENV !== "production") {
attachLoggerConsoleTransport(logger);
attachLoggerConsoleTransport(rootLogger);
}

if (typeof window === "undefined") {
import("@saleor/apps-logger/node").then(
async ({ attachLoggerOtelTransport, attachLoggerSentryTransport }) => {
const loggerContext = await import("./logger-context").then((m) => m.loggerContext);

attachLoggerSentryTransport(logger);
attachLoggerOtelTransport(logger, packageJson.version, loggerContext);
attachLoggerSentryTransport(rootLogger);
attachLoggerOtelTransport(rootLogger, packageJson.version, loggerContext);
},
);
}

export { createLogger, logger };
export const createLogger = (name: string, params?: Record<string, unknown>) =>
rootLogger.getSubLogger(
{
name: name,
},
params,
);
2 changes: 1 addition & 1 deletion packages/logger/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { createLogger, logger } from "./src/logger";
export { attachLoggerConsoleTransport } from "./src/logger-console-transport";
export { rootLogger } from "./src/root-logger";
Loading

0 comments on commit d088ef3

Please sign in to comment.