feat(health): normalized, actionable health-check errors (RFC #110) - #111
Draft
yesoreyeram wants to merge 5 commits into
Draft
feat(health): normalized, actionable health-check errors (RFC #110)#111yesoreyeram wants to merge 5 commits into
yesoreyeram wants to merge 5 commits into
Conversation
Introduce a new `health` module that normalizes data source CheckHealth failures into a consistent, safe, machine-classified and actionable result: a stable error code, a secret-free message, context-aware remediation and a redacted verbose detail. The core depends only on grafana-plugin-sdk-go. Provider-specific knowledge is injected by family libraries via RegisterClassifier / RegisterRules. Includes: - closed error-code taxonomy with canonical copy and default error source - classification pipeline: explicit tags, family classifiers (priority-ranked), generic net/TLS/timeout inspection, cancellation handling - response-aware handling for HTML interception and inconsistent JSON envelopes - secret redaction and safe body summaries - specificity-ranked remediation rules - single classify -> render to UI / logs / metrics / trace span - table-driven unit tests plus httptest-backed simulated scenarios (HTML gateway, SSO redirect, WAF block, JSON error, refused, timeout, cancel) Reference implementation for the RFC in #110.
Add an external (health_test) suite that exercises the module exactly as a plugin would, covering the two caller concerns: - context propagation: a sampled trace becomes the correlation id, an unsampled trace falls back to a generated id, and ctx deadline/cancellation flow through a model CheckHealth into the classified result. - error wrapping: tags survive deep fmt.Errorf wrapping, family classifiers match wrapped typed SDK errors via errors.As, wrapped sentinels classify via errors.Is, and secrets in wrapped driver errors never reach the message. Also make tag selection precedence-aware: when an error tree carries multiple tags (e.g. errors.Join of independent failures), Diagnose now picks the most actionable code (config > TLS > auth > connection > permission) instead of traversal order.
Add httpStatus to the JSONDetails contract so the upstream HTTP status code reaches the UI for reference alongside errorCode and providerCode. Populated from Diagnosis.HTTPStatus; omitted for non-HTTP failures (TLS/DNS/timeout).
Surface the diagnostic sub-signals (tlsKind, timeoutKind, bodyKind, contentType) in JSONDetails and fold the offending config field into remediation.fields, so the UI can render a complete structured description and highlight the relevant fields without consulting logs. All fields are optional and omitted when not applicable.
…rbose Revert the top-level httpStatus/tlsKind/timeoutKind/bodyKind/contentType fields added for self-description. The JSONDetails schema is restored to its fixed shape (errorCode, providerCode, errorSource, correlationId, remediation, verbose); the diagnostic metadata is now appended to the redacted verbose string instead, so the contract never grows new keys and the existing verbose message is preserved as the prefix. The offending field continues to surface via remediation.fields.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions! |
Collaborator
Author
|
Work in progress |
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
2 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 this adds
A new
healthmodule (github.com/grafana/dsconfig/health) that normalizes data sourceCheckHealthfailures (the errors behind Save & test) into a consistent, safe, machine-classified and actionable result:The core depends only on
grafana-plugin-sdk-go(per ADR-001). Provider knowledge (AWS/Azure/SQL/…) is injected by family libraries viaRegisterClassifier/RegisterRules— the core never imports those SDKs.Design ↔ code
code.goerrors.godiagnosis.goclassify.gohttpresponse.goredact.goremediation.goresult.goEntry points
Result(ctx, err, opts...)— classify a transport/driver error.ResultForResponse(ctx, resp, body, rawErr, opts...)— response-aware, so HTML/odd bodies classify correctly with sub-signals preserved.OK(message)— success constructor.Diagnose,ClassifyHTTPResponse,ExtractJSONError— exported building blocks.WithContext/WithLogger/WithMetrics/WithSpanRecorder/WithVerbose/WithDuration— sinks are injected, keeping the core dependency-light.Observability follows "classify once, render three ways": the safe payload is returned, while a redacted structured log line (severity keyed on error source), bounded-cardinality metrics, and a redacted span record fire only when their sinks are provided. The correlation ID is the trace ID when a sampled trace is present, else a generated fallback.
Tests — simulated scenarios
Table-driven unit tests plus
httptest-backed end-to-end scenarios that stand up a live server and run a modelCheckHealththrough it:UPSTREAM_ERRORAUTHENTICATION_FAILEDPERMISSION_DENIEDINVALID_CONFIGURATION+ provider code, hint in verboseOKHOST_UNREACHABLECONNECTION_TIMEOUTUNKNOWNstatus (not surfaced as an error)Plus units for generic classification (DNS/TLS/timeout variants), envelope extraction (Prometheus, Elastic, JSON:API, GraphQL, OAuth), redaction (URL creds, secret kv, bearer tokens, truncation), remediation specificity, and a runnable family-registration example.
Notes for reviewers
/healthvs a package in the existing/dsconfigmodule) is still an open question in the RFC — happy to relocate.SpanRecorderrather than importing OpenTelemetry directly, to keep the core's dependency to just the SDK.verboseUI gating, telemetry destination) and are intentionally not implemented here.Closes nothing yet — tracks #110.
Generated by Claude Code