fix: separate console and diagnostics client logging formats#611
Merged
daniyelnnr merged 4 commits intomasterfrom Oct 8, 2025
Merged
fix: separate console and diagnostics client logging formats#611daniyelnnr merged 4 commits intomasterfrom
daniyelnnr merged 4 commits intomasterfrom
Conversation
This update modifies the Logger class to streamline the construction of log objects, allowing to support legacy logging approach and the new one based on diagnostics
silvadenisaraujo
approved these changes
Oct 6, 2025
Contributor
silvadenisaraujo
left a comment
silvadenisaraujo
left a comment
There was a problem hiding this comment.
Agree with comments from @juliobguedes , besides LGTM!
This commit addresses PR review feedback by refactoring the logger to improve code maintainability and prevent data duplication between console and diagnostics client logs. Key changes: - Created commonLogFields object containing shared fields: __VTEX_IO_LOG, level, operationId, requestId, and traceId - Removed data field from diagnosticsLog by keeping it only in inflatedLog - Spread commonLogFields at the end of both log objects to maintain key order The refactoring ensures: 1. Console logs (inflatedLog) include all fields needed for fluent-bit/OpenSearch 2. Diagnostics client logs (diagnosticsLog) use semantic convention keys without data duplication 3. Common fields are defined once and reused consistently across both log formats Addresses review comments from @juliobguedes: - Comment 1: Remove data from diagnosticsLog to avoid duplications - Comment 2: Extract commonLogFields to reduce code duplication
This change adds status code as an attribute for some metrics like request timings and responses sizes
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What is the purpose of this pull request?
This PR fixes wrong mapping for some attributes, leading to fluent-bit being unable to create some attributes on OpenSearch, even though logs objects were being sent.
What problem is this solving?
#579 introduced a small bug in log attribute mapping. Attributes like "account," "workspace," and "appId" were present in the object exported to stdout but weren't being mapped correctly in the OpenSearch attributes view. An example is the image below, where we see a gap in the number of logs when filtering by one of the attributes with the affected mapping (you can also see that the issue was reversed after the rollback).
This issue was resolved by separating the object exported to stdout and logged to OpenSearch from the object logged by the diagnostics library client.
How should this be manually tested?
Screenshots or example usage
Types of changes