Instrumentation: Improve automatic instrumentation by the SDK to include handler/request logs, metrics and traces #1028
Conversation
marefr
requested review from
wbrowne,
andresmgot,
oshirohugo and
xnyo
and removed request for
a team
marefr
changed the title
backend/adapter_utils.go
Outdated
| } | ||
|
|
||
| // TODO status source | ||
| // logParams = append(logParams, "statusSource", pluginrequestmeta.StatusSourceFromContext(ctx)) |
There was a problem hiding this comment.
do you want to keep this?
There was a problem hiding this comment.
Related to description
Note 2) logs, metrics and traces currently lacks error source (status source in grafana) denoting whether an error originates from the plugin itself or from downstream service. There's some additional work needed to support this for all endpoints and would need some additional thinking to simplify for plugin authors.
I guess can target that in a follow up though. WDYT?
There was a problem hiding this comment.
yes, I just wanted to make sure you want to keep this commented code.
There was a problem hiding this comment.
Added support for error source (status source) to make it in pair with how it works in Grafana, however if a plugin returns a backend.DownstreamError(...) from CheckHealth or CallResource it will be recorded as a downstream error source. This should be enough for now and support the very most basic stuff.
Realized that having the ErrorSource in the backend package is not optimal. Tried to move the context support for error source to experimental/errorsource, but that creates a circular dependency 😢 So the current state of error status seems a bit broken to be used both internally in the SDK and in plugins. We can hopefully revisit in the future and introduce some breaking changes and clear up the problems.
|
Opened https://github.com/grafana/test-datasource/pull/39 seems to work as expected. |
Makes the adapters share some common functionality (middlewares).
backend.DownstreamError(...)orbackend.WithDownstreamErrorSource(...) could be returned/used fromCheckHealthandCallResource`, but should be considered experimental.RequestStatusfrom grafana into backend package for convenience (could be used by Grafana later)Note 1) There's #1019 which I had in mind for this. However, I think we can pick that up later and rather focusing on getting this logic in place.
Reminder: https://grafana.com/developers/plugin-tools/how-to-guides/data-source-plugins/add-logs-metrics-traces-for-backend-plugins should be updated later in regards to automatic instrumentation by the SDK.