-
Notifications
You must be signed in to change notification settings - Fork 201
fix(logger): pino pagerduty transport #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
Draft
Reinis-FRP
wants to merge
12
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.
Draft
+159
−62
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>
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
Explain the motivation for making this change.
What existing problem does the pull request solve?
It may be sufficient to just link to an issue, but please explain if more details are needed.
Summary
Briefly summarize what changes were made to accomplish the motivation above.
Details
This may be unnecessary for some PRs. Catch-all for detailed explanations about the implementation decisions and implications of the change.
Testing
Check a box to describe how you tested these changes and list the steps for reviewers to test.
Issue(s)
Fixes #XXXX