-
Notifications
You must be signed in to change notification settings - Fork 202
fix(logger): add Pino PagerDuty V2 transport support #4914
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
Open
Reinis-FRP
wants to merge
18
commits into
master
Choose a base branch
from
reinis-frp/pino-pd
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
- Create shared PagerDuty config module (types, validation, level conversion) - Implement PagerDutyV2Transport for Pino using pino-abstract-transport - Refactor Winston PagerDutyV2Transport to use shared config - Add createPinoTransports with conditional PagerDuty integration - Fix log level handling (read from env/config instead of hardcoded) - Update createPinoLogger to pass level to transports - Export Transports from logger package index - Match Winston's fail-fast behavior for config validation WIP: needs review and testing before finalizing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Change from inline type imports (import { type Foo }) to separate
import type statements to match project's linter configuration.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Remove re-exports of Config, createConfig, and types from transport files. These were only used internally and are now imported directly from the SharedConfig module by consumers (Transports.ts files). This makes it clear that these are shared utilities, not transport-specific APIs, and prevents them from being inadvertently exposed in the public API. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
…sConfig The environment parameter was copied from Winston's interface but is not needed for Pino. Unlike Winston, Pino doesn't require different transports for different environments - it always outputs JSON to stdout which GCP automatically parses. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
The previous implementation created a local variable that was never used, so the unmodified mrkdwn was being sent to PagerDuty. Now it directly mutates obj.mrkdwn to match the Winston implementation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Extract the PagerDuty event() call to a shared sendPagerDutyEvent() helper that accepts the whole log object and routing key. The helper now handles field extraction (level, message, at, botIdentifier) with fallbacks for both Winston and Pino log formats. This eliminates duplication and simplifies both transports - they now just call sendPagerDutyEvent(routing_key, logObj) instead of manually extracting and passing individual fields. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Move the removeAnchorTextFromLinks call into the shared sendPagerDutyEvent helper. This further simplifies both transports and ensures markdown is always cleaned consistently before sending to PagerDuty. Both transports are now extremely simple - just get the routing key and call sendPagerDutyEvent(routing_key, logObj). All field extraction, markdown cleaning, and event formatting is handled by the shared helper. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
…nsport.ts Reorganize PagerDuty shared utilities from pagerduty/SharedConfig.ts to shared/PagerDutyV2Transport.ts. This structure: - Matches naming convention of the transport files - Allows shared/ directory to contain other transport utilities - Eliminates the single-file pagerduty/ directory - Clearly indicates these are shared PagerDuty V2 utilities Updated all imports from ../pagerduty/SharedConfig to ../shared/PagerDutyV2Transport. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Remove unnecessary intermediate variables and fallbacks from sendPagerDutyEvent. Directly use logObj fields in payload construction to match original Winston implementation behavior. This preserves the original behavior where missing required fields appear as "undefined" in PagerDuty, making bugs in log formatting obvious. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
- Remove parse: "lines" option which was causing logs to be passed as strings instead of parsed objects
- Use Pino's levels.labels to convert numeric levels (50) to strings ("error") in summary
- Always log transport errors to console in Pino (no callback mechanism like Winston)
This fixes the issue where Pino logs were not being sent to PagerDuty due to incorrect parsing.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Configure Pino with stdSerializers.err to properly serialize Error objects with type, message, and stack properties. This matches Winston's errorStackTracerFormatter behavior and ensures errors are logged correctly instead of appearing as empty objects. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Replace manual numeric range checks with Pino's levels.labels mapping to convert numeric levels to strings. This is cleaner, more maintainable, and reuses Pino's existing infrastructure. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Fix convertLevelToSeverity to map unmapped levels (debug, trace) to "info" (lowest PagerDuty severity) instead of "error" (high severity). This bug was previously hidden because PagerDuty transports filter at level: "error", so debug/trace logs never reached the function in practice. This ensures correct severity mapping if transport level filtering is changed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Add unit tests for: - Shared utilities (createConfig, convertLevelToSeverity) - Winston PagerDutyV2Transport (routing keys, error handling, callbacks) - Pino PagerDutyV2Transport (stream processing, routing keys, error handling) Tests verify correct behavior for both Winston string levels and Pino numeric levels, custom routing keys via notificationPath, and proper error handling. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Revert discord-ticket-api server.ts changes to master version. These changes will be submitted in a separate PR. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
c0cad53 to
9f838c1
Compare
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The new
discord-ticket-apipackage uses Fastify with Pino logger. The existing PagerDuty V2 transport only supported Winston, so we needed to add Pino support to maintain consistent PagerDuty alerting across all services that use the@uma/loggerpackage.Summary
pino-abstract-transportshared/PagerDutyV2Transport.tsto eliminate code duplication between Winston and Pino implementationsstdSerializers.err) to Pino logger config for proper Error object serializationDetails
Implementation:
pinoLogger/PagerDutyV2Transport.ts): Stream-based transport running in worker thread, processes newline-delimited JSON logsshared/PagerDutyV2Transport.ts): ContainsConfigtype,createConfigvalidation,convertLevelToSeveritymapping, andsendPagerDutyEventhelperlogger/PagerDutyV2Transport.ts): Updated to use shared utilities, no functional changes to production behaviorKey technical decisions:
levels.labelsmapping to convert numeric levels (50 = error, 40 = warn, etc.) to string names"info"(lowest PagerDuty severity) instead of"error"to maintain semantic correctnesslevel: "error"so only error and fatal logs reach PagerDutyFiles modified:
packages/logger/src/pinoLogger/PagerDutyV2Transport.ts- New Pino transportpackages/logger/src/pinoLogger/Logger.ts- Added error serializerspackages/logger/src/pinoLogger/Transports.ts- Added PagerDuty configpackages/logger/src/shared/PagerDutyV2Transport.ts- New shared utilities modulepackages/logger/src/logger/PagerDutyV2Transport.ts- Refactored to use shared utilitiesTesting
Issue(s)
Part of the work to support Pino logger in new services.