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

Replace Logger with CommonLogger #233

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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: 13 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
# Upgrading Guide

## Upgrading from `17.0.0` to `18.0.0`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

core is updated to 18.0.0 but sns and amqp are updated to different versions, should we indicate that here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think breaking change is the same for all libraries, right?
Maybe we can do something like:

- mqt/core from 17.0.0 to 18.0.0 
- amqp from x to y 
- ...


### Description of Breaking Changes
- `logger` injectable dependency interface was replaced by `CommonLogger` from `@lokalise/node-core` package, It is compatible with on `pino` logger (https://github.com/pinojs/pino)

### Migration steps
- If you are using `pino` logger, or any other logger compatible with `CommonLogger` interface, you don't need to do anything
- Otherwise, there are serveral properties that need to be provided compared to the old interface:
- `level` - string defining configured minimal logging level
- `isLevelEnabled` - utility method for determining if a given log level will write to the destination.
- `child` - method for generating child logger instance with predefined properties
- `silent` - method used for logging when `silent` level is selected

## Upgrading from `12.0.0` to `13.0.0`

### Description of Breaking Change
Expand Down
6 changes: 3 additions & 3 deletions packages/amqp/lib/AmqpConnectionManager.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Logger } from '@message-queue-toolkit/core'
import type { Connection } from 'amqplib'

import type { CommonLogger } from '@lokalise/node-core'
import type { AmqpConfig } from './amqpConnectionResolver'
import { resolveAmqpConnection } from './amqpConnectionResolver'

Expand All @@ -11,14 +11,14 @@ export type ConnectionReceiver = {

export class AmqpConnectionManager {
private readonly config: AmqpConfig
private readonly logger: Logger
private readonly logger: CommonLogger
private readonly connectionReceivers: ConnectionReceiver[]
private connection?: Connection
public reconnectsActive: boolean

public isReconnecting: boolean

constructor(config: AmqpConfig, logger: Logger) {
constructor(config: AmqpConfig, logger: CommonLogger) {
this.config = config
this.connectionReceivers = []
this.reconnectsActive = true
Expand Down
4 changes: 2 additions & 2 deletions packages/amqp/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@message-queue-toolkit/amqp",
"version": "16.2.0",
"version": "17.0.0",
"private": false,
"license": "MIT",
"description": "AMQP adapter for message-queue-toolkit",
Expand All @@ -25,7 +25,7 @@
"prepublishOnly": "npm run build:release"
},
"dependencies": {
"@lokalise/node-core": "^13.1.0",
"@lokalise/node-core": "^13.3.0",
CarlosGamero marked this conversation as resolved.
Show resolved Hide resolved
"zod": "^3.23.8"
},
"peerDependencies": {
Expand Down
28 changes: 23 additions & 5 deletions packages/amqp/test/fakes/FakeLogger.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { deepClone } from '@lokalise/node-core'
import type { Logger } from '@message-queue-toolkit/core'
import { type CommonLogger, deepClone } from '@lokalise/node-core'
import type { Bindings, ChildLoggerOptions } from 'pino'
import type pino from 'pino'

export class FakeLogger implements Logger {
export class FakeLogger implements CommonLogger {
public readonly loggedMessages: unknown[] = []
public readonly loggedWarnings: unknown[] = []
public readonly loggedErrors: unknown[] = []

public level: pino.LevelWithSilentOrString

constructor(level: pino.LevelWithSilentOrString = 'debug') {
this.level = level
}

debug(obj: unknown) {
this.saveLog(obj)
Expand All @@ -24,6 +29,19 @@ export class FakeLogger implements Logger {
warn(obj: unknown) {
this.saveLog(obj)
}
silent(_obj: unknown) {
return
}

// Child has no effect for FakeLogger
child(_bindings: Bindings, _options?: ChildLoggerOptions): CommonLogger {
return this
}

isLevelEnabled(_: pino.LevelWithSilentOrString): boolean {
// For FakeLogger we want to track all logs
return true
}

private saveLog(obj: unknown) {
this.loggedMessages.push(typeof obj === 'object' ? deepClone(obj) : obj)
Expand Down
8 changes: 4 additions & 4 deletions packages/amqp/test/utils/testContext.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ErrorReporter, ErrorResolver } from '@lokalise/node-core'
import type { Logger, TransactionObservabilityManager } from '@message-queue-toolkit/core'
import type { CommonLogger, ErrorReporter, ErrorResolver } from '@lokalise/node-core'
import type { TransactionObservabilityManager } from '@message-queue-toolkit/core'
import {
CommonMetadataFiller,
EventRegistry,
Expand Down Expand Up @@ -81,7 +81,7 @@ export type TestEventsType = (typeof TestEvents)[keyof typeof TestEvents][]
export type TestEventPublishPayloadsType = z.infer<TestEventsType[number]['publisherSchema']>

// @ts-ignore
const TestLogger: Logger = console
const TestLogger: CommonLogger = console

export async function registerDependencies(
config: AmqpConfig,
Expand Down Expand Up @@ -202,7 +202,7 @@ export async function registerDependencies(
type DiConfig = NameAndRegistrationPair<Dependencies>

export interface Dependencies {
logger: Logger
logger: CommonLogger
amqpConnectionManager: AmqpConnectionManager
awilixManager: AwilixManager

Expand Down
2 changes: 0 additions & 2 deletions packages/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ export type {
AsyncPublisher,
SyncPublisher,
TransactionObservabilityManager,
Logger,
LogFn,
SchemaMap,
ExtraParams,
} from './lib/types/MessageQueueTypes'
Expand Down
6 changes: 3 additions & 3 deletions packages/core/lib/events/DomainEventEmitter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
type CommonLogger,
type ErrorReporter,
InternalError,
type TransactionObservabilityManager,
Expand All @@ -11,7 +12,6 @@ import { resolveHandlerSpy } from '../queues/HandlerSpy'

import { randomUUID } from 'node:crypto'
import type { ConsumerMessageMetadataType } from '@message-queue-toolkit/schemas'
import type { Logger } from '../types/MessageQueueTypes'
import type { EventRegistry } from './EventRegistry'
import type {
AnyEventHandler,
Expand All @@ -26,7 +26,7 @@ import type {
export type DomainEventEmitterDependencies<SupportedEvents extends CommonEventDefinition[]> = {
eventRegistry: EventRegistry<SupportedEvents>
metadataFiller: MetadataFiller
logger: Logger
logger: CommonLogger
errorReporter?: ErrorReporter
transactionObservabilityManager?: TransactionObservabilityManager
}
Expand All @@ -39,7 +39,7 @@ type Handlers<T> = {
export class DomainEventEmitter<SupportedEvents extends CommonEventDefinition[]> {
private readonly eventRegistry: EventRegistry<SupportedEvents>
private readonly metadataFiller: MetadataFiller
private readonly logger: Logger
private readonly logger: CommonLogger
private readonly errorReporter?: ErrorReporter
private readonly transactionObservabilityManager?: TransactionObservabilityManager
private readonly _handlerSpy?: HandlerSpy<
Expand Down
10 changes: 7 additions & 3 deletions packages/core/lib/queues/AbstractQueueService.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { types } from 'node:util'

import type { Either, ErrorReporter, ErrorResolver } from '@lokalise/node-core'
import type { CommonLogger, Either, ErrorReporter, ErrorResolver } from '@lokalise/node-core'
import { resolveGlobalErrorLogObject } from '@lokalise/node-core'
import type { CommonEventDefinition } from '@message-queue-toolkit/schemas'
import type { ZodSchema, ZodType } from 'zod'
Expand All @@ -11,7 +11,7 @@ import { OFFLOADED_PAYLOAD_POINTER_PAYLOAD_SCHEMA } from '../payload-store/offlo
import type { OffloadedPayloadPointerPayload } from '../payload-store/offloadedPayloadMessageSchemas'
import type { PayloadStoreConfig } from '../payload-store/payloadStoreTypes'
import { isDestroyable } from '../payload-store/payloadStoreTypes'
import type { Logger, MessageProcessingResult } from '../types/MessageQueueTypes'
import type { MessageProcessingResult } from '../types/MessageQueueTypes'
import type { DeletionConfig, QueueDependencies, QueueOptions } from '../types/queueOptionsTypes'
import { isRetryDateExceeded } from '../utils/dateUtils'
import { streamWithKnownSizeToString } from '../utils/streamUtils'
Expand Down Expand Up @@ -69,7 +69,7 @@ export abstract class AbstractQueueService<
protected readonly messageTimestampField: string

protected readonly errorReporter: ErrorReporter
public readonly logger: Logger
public readonly logger: CommonLogger
protected readonly messageIdField: string
protected readonly messageTypeField: string
protected readonly logMessages: boolean
Expand Down Expand Up @@ -169,6 +169,10 @@ export abstract class AbstractQueueService<
processingResult: MessageProcessingResult,
messageId?: string,
) {
if (!this.logger.isLevelEnabled('debug')) {
return
}

const messageTimestamp = message ? this.tryToExtractTimestamp(message) : undefined
const messageProcessingMilliseconds = messageTimestamp
? Date.now() - messageTimestamp.getTime()
Expand Down
22 changes: 2 additions & 20 deletions packages/core/lib/types/MessageQueueTypes.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { TransactionObservabilityManager } from '@lokalise/node-core'
import type { CommonLogger, TransactionObservabilityManager } from '@lokalise/node-core'
import type { ZodSchema } from 'zod'

import type { PublicHandlerSpy } from '../queues/HandlerSpy'
Expand Down Expand Up @@ -27,26 +27,8 @@ export interface AsyncPublisher<MessagePayloadType extends object, MessageOption

export type { TransactionObservabilityManager }

export type LogFn = {
// biome-ignore lint/suspicious/noExplicitAny: This is expected
<T extends object>(obj: T, msg?: string, ...args: any[]): void
// biome-ignore lint/suspicious/noExplicitAny: This is expected
(obj: unknown, msg?: string, ...args: any[]): void
// biome-ignore lint/suspicious/noExplicitAny: This is expected
(msg: string, ...args: any[]): void
}

export type ExtraParams = {
logger?: Logger
}

export type Logger = {
error: LogFn
info: LogFn
warn: LogFn
debug: LogFn
trace: LogFn
fatal: LogFn
logger?: CommonLogger
}

export type SchemaMap<SupportedMessageTypes extends string> = Record<
Expand Down
6 changes: 3 additions & 3 deletions packages/core/lib/types/queueOptionsTypes.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import type { ErrorReporter, ErrorResolver } from '@lokalise/node-core'
import type { CommonLogger, ErrorReporter, ErrorResolver } from '@lokalise/node-core'
import type { ZodSchema } from 'zod'

import type { PayloadStoreConfig } from '../payload-store/payloadStoreTypes'
import type { MessageHandlerConfig } from '../queues/HandlerContainer'
import type { HandlerSpy, HandlerSpyParams } from '../queues/HandlerSpy'

import type { Logger, TransactionObservabilityManager } from './MessageQueueTypes'
import type { TransactionObservabilityManager } from './MessageQueueTypes'

export type QueueDependencies = {
errorReporter: ErrorReporter
logger: Logger
logger: CommonLogger
}

export type QueueConsumerDependencies = {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@message-queue-toolkit/core",
"version": "17.2.3",
"version": "18.0.0",
"private": false,
"license": "MIT",
"description": "Useful utilities, interfaces and base classes for message queue handling. Supports AMQP and SQS with a common abstraction on top currently",
Expand All @@ -25,7 +25,7 @@
"prepublishOnly": "npm run build:release"
},
"dependencies": {
"@lokalise/node-core": "^13.0.1",
"@lokalise/node-core": "^13.3.0",
"@message-queue-toolkit/schemas": "^4.0.0",
"fast-equals": "^5.0.1",
"json-stream-stringify": "^3.1.6",
Expand Down
26 changes: 0 additions & 26 deletions packages/core/test/fakes/FakeLogger.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ export class FakeTransactionObservabilityManager implements TransactionObservabi
startWithGroup() {}

stop() {}

addCustomAttributes() {}
}
7 changes: 3 additions & 4 deletions packages/core/test/testContext.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import type { ErrorReporter } from '@lokalise/node-core'
import type { CommonLogger, ErrorReporter } from '@lokalise/node-core'
import { type Resolver, asClass } from 'awilix'
import { Lifetime, asFunction, createContainer } from 'awilix'
import { AwilixManager } from 'awilix-manager'
import type { Logger } from 'pino'
import pino from 'pino'
import { z } from 'zod'

Expand All @@ -19,7 +18,7 @@ export const SINGLETON_CONFIG = { lifetime: Lifetime.SINGLETON }

export type DependencyOverrides = Partial<DiConfig>

const TestLogger: Logger = pino()
const TestLogger: CommonLogger = pino()

export const TestEvents = {
created: {
Expand Down Expand Up @@ -100,7 +99,7 @@ export async function registerDependencies(dependencyOverrides: DependencyOverri
type DiConfig = Record<keyof Dependencies, Resolver<any>>

export interface Dependencies {
logger: Logger
logger: CommonLogger
awilixManager: AwilixManager

// vendor-specific dependencies
Expand Down
4 changes: 2 additions & 2 deletions packages/sns/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@message-queue-toolkit/sns",
"version": "18.1.0",
"version": "19.0.0",
"private": false,
"license": "MIT",
"description": "SNS adapter for message-queue-toolkit",
Expand All @@ -25,7 +25,7 @@
"prepublishOnly": "npm run build:release"
},
"dependencies": {
"@lokalise/node-core": "^13.0.1",
"@lokalise/node-core": "^13.3.0",
"sqs-consumer": "^11.1.0",
CarlosGamero marked this conversation as resolved.
Show resolved Hide resolved
"zod": "^3.23.8"
},
Expand Down
26 changes: 24 additions & 2 deletions packages/sns/test/fakes/FakeLogger.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import type { Logger } from '@message-queue-toolkit/core'
import type { CommonLogger } from '@lokalise/node-core'
import type { Bindings, ChildLoggerOptions } from 'pino'
import type pino from 'pino'

export class FakeLogger implements Logger {
export class FakeLogger implements CommonLogger {
public readonly loggedMessages: unknown[] = []
public readonly loggedWarnings: unknown[] = []
public readonly loggedErrors: unknown[] = []

public readonly level: pino.LevelWithSilentOrString

constructor(level: pino.LevelWithSilentOrString = 'debug') {
this.level = level
}

debug(obj: unknown) {
this.loggedMessages.push(obj)
}
Expand All @@ -23,4 +31,18 @@ export class FakeLogger implements Logger {
warn(obj: unknown) {
this.loggedWarnings.push(obj)
}
// Noop function
silent(_obj: unknown) {
return
}

// Child has no effect for FakeLogger
child(_bindings: Bindings, _options?: ChildLoggerOptions): CommonLogger {
return this
}

isLevelEnabled(_level: string): boolean {
// For FakeLogger we want to track all logs
return true
}
}
Loading
Loading