Plugins: Fix status_source always being "plugin" in plugin request logs #77433
Conversation
This file contains 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
xnyo
added
type/bug
area/plugins
area/backend/plugins
area/backend
add to changelog
go
xnyo
requested review from
wbrowne,
marefr and
oshirohugo
and removed request for
a team
|
Thank you @xnyo for looking into this and fixing it! 🧡 |
xnyo
commented
Oct 31, 2023
Comment on lines
+35
to
+56
| // Set downstream status source in the context if there's at least one response with downstream status source, | ||
| // and if there's no plugin error | ||
| var hasPluginError bool | ||
| var hasDownstreamError bool | ||
| for _, r := range resp.Responses { | ||
| if r.Error == nil { | ||
| continue | ||
| } | ||
| if r.ErrorSource == backend.ErrorSourceDownstream { | ||
| hasDownstreamError = true | ||
| } else { | ||
| hasPluginError = true | ||
| } | ||
| } | ||
|
|
||
| // A plugin error has higher priority than a downstream error, | ||
| // so set to downstream only if there's no plugin error | ||
| if hasDownstreamError && !hasPluginError { | ||
| if err := pluginrequestmeta.WithDownstreamStatusSource(ctx); err != nil { | ||
| return resp, fmt.Errorf("failed to set downstream status source: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
This is the same logic as before, but it has been moved from MetricsMiddleware
3 tasks
xnyo
added a commit
that referenced
this pull request
Oct 31, 2023
…request logs (#77436) Plugins: Fix status_source always being "plugin" in plugin request logs (#77433) * Plugins: Fix status_source always being "plugin" in plugin logs * add tests * Fix TestInstrumentationMiddlewareStatusSource (cherry picked from commit 46261de) Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com>
ssama88
pushed a commit
to ssama88/grafana
that referenced
this pull request
Oct 31, 2023
…gs (grafana#77433) * Plugins: Fix status_source always being "plugin" in plugin logs * add tests * Fix TestInstrumentationMiddlewareStatusSource
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.
What is this feature?
Fixes a bug where
status_sourcein plugin request logs is alwaysplugin, even if it was set todownstreamby a plugin.This happened because the logic for the status source was all contained in the metrics middleware, and the logger middleware is at a deeper level:
This PR adds two new middlewares:
PluginRequestMetaMiddleware, which is at the top of the middleware stack, creates the plugin request meta in thecontext.Context, which is then available to all subsequent middlewaresStatusSourceMiddleware, at the bottom of the middleware stack, which sets the status source in thecontext.Contextbefore returning, so all middlewares above it can access the correct status source after QueryData has returnedWhy do we need this feature?
Bugfix
Who is this feature for?
Grafana operators
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: