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

chore: add extra logging for by-slice queries #114

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Conversation

pvlugter
Copy link
Contributor

No description provided.

@pvlugter pvlugter force-pushed the query-extra-logging branch from 39b1679 to 04633cb Compare December 11, 2024 00:14
Source
.fromPublisher(publisher)
.log(logName, logQueryResponse)(logging)
Copy link
Member

Choose a reason for hiding this comment

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

Could we propagate the logPrefix to the dao so that we try to have the same format, or at least similar prefix format here?

s"[$entityType] eventsBySlice [$slice]: "

Is there some added value of using stream log or shall we just add ordinary logging inside the mapConcat?
For example, stream fail will be logged at error level if we don't adjust the levels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update the log prefix to be the same pattern. Yes, using the stream logging intentional here, so we can see the completions in particular. We could adjust failures to be debug as well, if we think error would be a problem.

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM

// note that this is not logging each item, only the QueryResponse
.log(logName, logQueryResponse)(logging)
.withAttributes(Attributes
.logLevels(onElement = Logging.DebugLevel, onFinish = Logging.DebugLevel, onFailure = Logging.WarningLevel))
Copy link
Member

Choose a reason for hiding this comment

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

setting the levels, warning instead of error

@patriknw patriknw merged commit 8bb8e6d into main Dec 12, 2024
5 checks passed
@patriknw patriknw deleted the query-extra-logging branch December 12, 2024 11:24
@patriknw patriknw modified the milestones: 2.0.5, 2.0.4 Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants