From 90894fb7155d3dea6ac7ed5ef2b99a39b4f9acac Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 3 Sep 2026 20:29:28 +0000 Subject: [PATCH 1/5] fix: resolve React hooks warnings, memoize projects, and fix Go proto marshaling --- .gitignore | 3 +- eslint.config.mjs | 9 +++++ pkg/plugin/cloudlogging/cloudlogging.go | 4 +- src/QueryEditor.tsx | 54 +++++++++++++++++-------- 4 files changed, 50 insertions(+), 20 deletions(-) create mode 100644 eslint.config.mjs diff --git a/.gitignore b/.gitignore index a31c3a9..b6f5321 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ node_modules dist/ -coverage/ \ No newline at end of file +coverage/ +.eslintcache diff --git a/eslint.config.mjs b/eslint.config.mjs new file mode 100644 index 0000000..50cdf5d --- /dev/null +++ b/eslint.config.mjs @@ -0,0 +1,9 @@ +import { defineConfig } from 'eslint/config'; +import baseConfig from './.config/eslint.config.mjs'; + +export default defineConfig([ + { + ignores: ['dist/**', 'node_modules/**', '.config/**', 'coverage/**', 'playwright-report/**', 'test-results/**'], + }, + ...baseConfig, +]); diff --git a/pkg/plugin/cloudlogging/cloudlogging.go b/pkg/plugin/cloudlogging/cloudlogging.go index cd9d92b..c0873d6 100644 --- a/pkg/plugin/cloudlogging/cloudlogging.go +++ b/pkg/plugin/cloudlogging/cloudlogging.go @@ -98,7 +98,7 @@ func GetLogLabels(entry *loggingpb.LogEntry) data.Labels { if err := t.ProtoPayload.UnmarshalTo(&a); err != nil { log.DefaultLogger.Error("Could not get AuditLog payload out of LogEntry", "error", err) } else { - byteArr, _ := json.Marshal(a) + byteArr, _ := json.Marshal(&a) var inInterface map[string]*structpb.Value json.Unmarshal(byteArr, &inInterface) for k, v := range inInterface { @@ -110,7 +110,7 @@ func GetLogLabels(entry *loggingpb.LogEntry) data.Labels { if err := t.ProtoPayload.UnmarshalTo(&r); err != nil { log.DefaultLogger.Error("Could not get RequestLog payload out of LogEntry", "error", err) } else { - byteArr, _ := json.Marshal(r) + byteArr, _ := json.Marshal(&r) var inInterface map[string]*structpb.Value json.Unmarshal(byteArr, &inInterface) for k, v := range inInterface { diff --git a/src/QueryEditor.tsx b/src/QueryEditor.tsx index 35d1836..55ed51c 100644 --- a/src/QueryEditor.tsx +++ b/src/QueryEditor.tsx @@ -34,9 +34,21 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu } }; - // Keep a ref to the latest query so async callbacks avoid stale closures + // Keep refs to the latest query and callbacks so async work in the + // init effect below avoids stale closures. Grafana recreates onChange and + // onRunQuery on every render, so they can't be effect dependencies without + // re-running the (expensive, side-effecting) init logic each render. + // Refs are synced in an effect rather than during render, as required by + // react-hooks/refs. Effects run in declaration order, so this one always + // runs before the init effect in the same commit. const queryRef = useRef(query); - queryRef.current = query; + const onChangeRef = useRef(onChange); + const onRunQueryRef = useRef(onRunQuery); + useEffect(() => { + queryRef.current = query; + onChangeRef.current = onChange; + onRunQueryRef.current = onRunQuery; + }); // Compute normalized queryText as a derived value (never mutate the prop directly) const effectiveQueryText = query.query ?? query.queryText ?? defaultQuery.queryText; @@ -67,7 +79,7 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu if (currentProjectId && currentProjectId.startsWith('$')) { if (!cancelled && Object.keys(textUpdates).length > 0) { - onChange({ ...latestQuery, ...textUpdates }); + onChangeRef.current({ ...latestQuery, ...textUpdates }); } return; } @@ -102,7 +114,7 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu updates.bucketId = ''; } if (Object.keys(updates).length > 0) { - onChange({ ...latestQuery, ...updates }); + onChangeRef.current({ ...latestQuery, ...updates }); } return; } @@ -127,9 +139,9 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu } if (cancelled) { return; } - onChange({ ...latestQuery, ...textUpdates, projectId: newProjectId, bucketId: '', viewId: '' }); + onChangeRef.current({ ...latestQuery, ...textUpdates, projectId: newProjectId, bucketId: '', viewId: '' }); if (newProjectId) { - onRunQuery(); + onRunQueryRef.current(); } })(); @@ -166,16 +178,26 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu uid: string | null; list: Array>; }>({ uid: null, list: [] }); - const [projectsLoading, setProjectsLoading] = useState(false); + // Search-triggered loads are tagged with the DS uid they were started for, + // so a stale in-flight search for a previous datasource never shows a + // spinner for the current one. + const [searchLoadingUid, setSearchLoadingUid] = useState(null); const searchTimer = useRef>(); - const projectsForCurrentDs = projectsState.uid === datasource.uid - ? projectsState.list - : []; + // Memoized so its identity is stable between renders; it is a dependency of + // the bucket-loading effect below. + const projectsForCurrentDs = useMemo( + () => (projectsState.uid === datasource.uid ? projectsState.list : []), + [projectsState, datasource.uid] + ); + // Derived rather than stored: the picker is loading whenever the loaded + // list doesn't belong to the current DS yet, or a search for the current + // DS is in flight. Storing this as state would require a synchronous + // setState inside the effect below (react-hooks/set-state-in-effect). + const projectsLoading = projectsState.uid !== datasource.uid || searchLoadingUid === datasource.uid; useEffect(() => { let cancelled = false; const loadingForUid = datasource.uid; - setProjectsLoading(true); datasource.getFilteredProjects() .then(res => { if (cancelled) { return; } @@ -189,9 +211,6 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu if (cancelled) { return; } setProjectsState({ uid: loadingForUid, list: [] }); setFetchError(sanitizeFetchError(err)); - }) - .finally(() => { - if (!cancelled) { setProjectsLoading(false); } }); return () => { cancelled = true; @@ -202,7 +221,7 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu const onProjectSearchChange = useCallback((value: string) => { if (searchTimer.current) { clearTimeout(searchTimer.current); } const searchDsUid = datasource.uid; - setProjectsLoading(true); + setSearchLoadingUid(searchDsUid); searchTimer.current = setTimeout(() => { datasource.getFilteredProjects(value || undefined) .then(res => { @@ -221,7 +240,8 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu setFetchError(sanitizeFetchError(err)); }) .finally(() => { - if (searchDsUid === datasource.uid) { setProjectsLoading(false); } + // Only clear the flag if no newer search superseded this one. + setSearchLoadingUid(current => (current === searchDsUid ? null : current)); }); }, 300); }, [datasource]); @@ -313,7 +333,7 @@ export function LoggingQueryEditor({ datasource, query, range, onChange, onRunQu } return `https://console.cloud.google.com/logs/query?${queryParams.join('&')}`; - }, [query, range]); + }, [query, range, effectiveQueryText]); return ( <> From 1874b10dbcb4e8cba305d55e0d9f5e5597c98a34 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 3 Sep 2026 21:15:32 +0000 Subject: [PATCH 2/5] fix issue #221 --- CHANGELOG.md | 5 + package.json | 2 +- pkg/plugin/cloudlogging/cloudlogging.go | 43 ++--- pkg/plugin/cloudlogging/cloudlogging_test.go | 49 +++--- pkg/plugin/plugin.go | 73 +++++++-- pkg/plugin/plugin_test.go | 155 ++++++++++++++++++- src/datasource.test.ts | 151 +++++++++++------- src/datasource.ts | 146 ++++++++++------- 8 files changed, 445 insertions(+), 179 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d51d2d2..5c190ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,9 @@ # Changelog +## 1.8.0 (2026-09-03) +* **Breaking: logs responses now use Grafana's dataplane `log-lines` format** ([#221](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/221)). Each query returns a single frame with one row per log entry instead of one frame per entry. Fields are `timestamp`, `body`, `severity`, `id` (the entry's insert ID), `labels` (per-row JSON object) and `traceId`; previously they were `time` and `content` with the metadata attached as labels on `content`. Grafana can now identify each log line uniquely, which fixes log details expanding every line at once, the log list jumping to the top on click, broken permalinks and dedup, and the Logs Table showing only the first line. Dashboards that reference the old `content` or `time` field names in transformations or Table panels need updating +* "View trace" links are resolved per log line. The project comes from each entry's own trace path (or the query/default project, as before), so a single result set spanning several projects links each line to the right trace +* An empty result now returns an empty frame with the logs schema instead of no frame + ## 1.7.2 (2026-08-17) * Update dependencies to address security vulnerabilities flagged by the Grafana plugin review: js-yaml (CVE-2026-59869) and nanoid (CVE-2026-67213) in the frontend build toolchain * Upgrade the Grafana Go plugin SDK from v0.290.0 to v0.296.2 diff --git a/package.json b/package.json index 385ee5b..dd1f739 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "googlecloud-logging-datasource", - "version": "1.7.2", + "version": "1.8.0", "description": "Backend Grafana plugin that enables visualization of GCP Cloud Logging logs in Grafana.", "scripts": { "build": "webpack -c ./.config/webpack/webpack.config.ts --env production", diff --git a/pkg/plugin/cloudlogging/cloudlogging.go b/pkg/plugin/cloudlogging/cloudlogging.go index c0873d6..ef590b5 100644 --- a/pkg/plugin/cloudlogging/cloudlogging.go +++ b/pkg/plugin/cloudlogging/cloudlogging.go @@ -62,17 +62,18 @@ func GetLogEntryMessage(entry *loggingpb.LogEntry) (string, error) { } } -// GetLogLabels flattens a log entry's labels + resource labels into a map +// GetLogLabels flattens a log entry's labels + resource labels into a map. +// +// The entry's insert ID, severity and bare trace ID are deliberately not +// included: they are emitted as dedicated `id`, `severity` and `traceId` +// frame fields (see plugin.go), and repeating them here would show them +// twice in Grafana's log details. func GetLogLabels(entry *loggingpb.LogEntry) data.Labels { labels := make(data.Labels) for k, v := range entry.GetLabels() { labels[fmt.Sprintf("labels.\"%s\"", k)] = v } - labels["id"] = entry.GetInsertId() - // This is how severity is set - labels["level"] = GetLogLevel(entry.GetSeverity()) - resource := entry.GetResource() if resourceType := resource.GetType(); resourceType != "" { labels["resource.type"] = resourceType @@ -136,26 +137,32 @@ func GetLogLabels(entry *loggingpb.LogEntry) data.Labels { // Add trace data. // Contract: the frontend's logs-to-traces feature (src/datasource.ts, - // addTraceLinkField) depends on the `trace` and `traceId` label names and - // on `trace` carrying the raw LogEntry value in the canonical - // `projects//traces/` form — it re-parses that path to - // extract the project for the "View trace" link. Renaming these labels or - // changing their format silently breaks that feature; no test crosses the - // Go/TS boundary. - traceId := entry.GetTrace() - spanId := entry.GetSpanId() - if traceId != "" { - trace := entry.GetTrace() + // addTraceLinkField) depends on the `trace` label carrying the raw + // LogEntry value in the canonical `projects//traces/` form — + // it re-parses that path per row to extract the project for the + // "View trace" link. Changing its name or format silently breaks that + // feature; no test crosses the Go/TS boundary. + if trace := entry.GetTrace(); trace != "" { labels["trace"] = trace - labels["traceId"] = strings.Split(trace, "/")[len(strings.Split(trace, "/"))-1] } - if spanId != "" { - labels["spanId"] = entry.GetSpanId() + if spanId := entry.GetSpanId(); spanId != "" { + labels["spanId"] = spanId } return labels } +// GetTraceID returns the bare trace ID of a log entry (the last path segment +// of LogEntry.trace, which is normally `projects//traces/`), or +// an empty string when the entry carries no trace. +func GetTraceID(entry *loggingpb.LogEntry) string { + trace := entry.GetTrace() + if trace == "" { + return "" + } + return trace[strings.LastIndex(trace, "/")+1:] +} + // GetLogLevel maps the string value of a LogSeverity to one supported by Grafana func GetLogLevel(severity ltype.LogSeverity) string { switch severity { diff --git a/pkg/plugin/cloudlogging/cloudlogging_test.go b/pkg/plugin/cloudlogging/cloudlogging_test.go index ef4914d..342b3b7 100644 --- a/pkg/plugin/cloudlogging/cloudlogging_test.go +++ b/pkg/plugin/cloudlogging/cloudlogging_test.go @@ -253,10 +253,7 @@ func TestGetLogLabels(t *testing.T) { entry: &loggingpb.LogEntry{ InsertId: "insert-id", }, - expected: data.Labels{ - "id": "insert-id", - "level": "info", - }, + expected: data.Labels{}, }, { name: "no log labels, but resource with labels", @@ -270,10 +267,8 @@ func TestGetLogLabels(t *testing.T) { }, }, expected: data.Labels{ - "id": "insert-id", "resource.labels.instance_id": "123456", "resource.type": "gce_instance", - "level": "info", }, }, { @@ -292,12 +287,10 @@ func TestGetLogLabels(t *testing.T) { }, }, expected: data.Labels{ - "id": "insert-id2", "labels.\"pid\"": "111", "labels.\"LOG_BUCKET_NUM\"": "1", "resource.labels.instance_id": "98765", "resource.type": "cloudsql_database", - "level": "info", }, }, { @@ -335,7 +328,6 @@ func TestGetLogLabels(t *testing.T) { }, }, expected: data.Labels{ - "id": "insert-id4", "labels.\"logging.googleapis.com/instrumentation_source\"": "agent.googleapis.com/thirdparty", "jsonPayload.tid": "222", "jsonPayload.db": "database-experiencing-error", @@ -343,7 +335,6 @@ func TestGetLogLabels(t *testing.T) { "labels.\"LOG_BUCKET_NUM\"": "1", "resource.labels.instance_id": "98765", "resource.type": "gce_instance", - "level": "alert", "jsonPayload.service_context.service": "some-service", "jsonPayload.service_context.version": "v42", }, @@ -357,8 +348,6 @@ func TestGetLogLabels(t *testing.T) { }, }, expected: data.Labels{ - "id": "insert-id5", - "level": "info", "textPayload": "This is a text log message", }, }, @@ -370,11 +359,8 @@ func TestGetLogLabels(t *testing.T) { SpanId: "000000000000004a", }, expected: data.Labels{ - "id": "insert-id6", - "level": "info", - "trace": "projects/my-project/traces/06796866738c859f2f19b7cfb3214824", - "traceId": "06796866738c859f2f19b7cfb3214824", - "spanId": "000000000000004a", + "trace": "projects/my-project/traces/06796866738c859f2f19b7cfb3214824", + "spanId": "000000000000004a", }, }, { @@ -401,8 +387,6 @@ func TestGetLogLabels(t *testing.T) { }, }, expected: data.Labels{ - "id": "insert-id7", - "level": "info", "jsonPayload.string_field": "test", "jsonPayload.number_field": "42.5", "jsonPayload.bool_field": "false", @@ -421,10 +405,7 @@ func TestGetLogLabels(t *testing.T) { }, }, }, - expected: data.Labels{ - "id": "insert-id8", - "level": "info", - }, + expected: data.Labels{}, }, { name: "Proto payload with RequestLog", @@ -437,10 +418,7 @@ func TestGetLogLabels(t *testing.T) { }, }, }, - expected: data.Labels{ - "id": "insert-id9", - "level": "info", - }, + expected: data.Labels{}, }, } @@ -451,6 +429,23 @@ func TestGetLogLabels(t *testing.T) { } } +func TestGetTraceID(t *testing.T) { + testCases := []struct { + name string + trace string + expected string + }{ + {name: "canonical resource path", trace: "projects/my-project/traces/06796866738c859f2f19b7cfb3214824", expected: "06796866738c859f2f19b7cfb3214824"}, + {name: "bare id", trace: "06796866738c859f2f19b7cfb3214824", expected: "06796866738c859f2f19b7cfb3214824"}, + {name: "no trace", trace: "", expected: ""}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, cloudlogging.GetTraceID(&loggingpb.LogEntry{Trace: tc.trace})) + }) + } +} + // normalizeLabelSpaces collapses runs of whitespace in label values so // assertions don't depend on protobuf text-format output, which inserts // unstable whitespace by design. diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 267037a..aa8d6cd 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -434,32 +434,73 @@ func (d *CloudLoggingDatasource) query(ctx context.Context, pCtx backend.PluginC return response } - // create data frame response. - frames := []*data.Frame{} - - for i := 0; i < len(logs); i++ { - body, err := cloudlogging.GetLogEntryMessage(logs[i]) + // Build a single dataplane "log-lines" frame with one row per entry + // (https://grafana.com/developers/dataplane/logs). Grafana identifies a + // log row by refId + the `id` field; the previous one-frame-per-entry + // layout left every row at index 0 of its own frame with no id, so all + // rows shared one uid and log details, permalinks and the Logs Table + // misbehaved (issue #221). + n := len(logs) + timestamps := make([]time.Time, 0, n) + bodies := make([]string, 0, n) + severities := make([]string, 0, n) + ids := make([]string, 0, n) + labels := make([]json.RawMessage, 0, n) + traceIDs := make([]*string, 0, n) + + for i, entry := range logs { + body, err := cloudlogging.GetLogEntryMessage(entry) if err != nil { // some log messages might not have a payload // log a warning here but continue log.DefaultLogger.Warn("failed getting log message", "warning", err) } - labels := cloudlogging.GetLogLabels(logs[i]) - f := data.NewFrame(logs[i].GetInsertId()) - timestamp := data.NewField("time", nil, []time.Time{logs[i].GetTimestamp().AsTime()}) - content := data.NewField("content", labels, []string{body}) + // The API always assigns an insert ID, but never let an empty one + // through: rows with equal ids collapse into one in Grafana. + id := entry.GetInsertId() + if id == "" { + id = fmt.Sprintf("%s_%d", query.RefID, i) + } + + entryLabels, err := json.Marshal(cloudlogging.GetLogLabels(entry)) + if err != nil { + log.DefaultLogger.Warn("failed encoding log labels", "warning", err) + entryLabels = json.RawMessage("{}") + } + + var traceID *string + if t := cloudlogging.GetTraceID(entry); t != "" { + traceID = &t + } - f.Fields = append(f.Fields, timestamp, content) - f.Meta = &data.FrameMeta{} - f.Meta.PreferredVisualization = data.VisTypeLogs - frames = append(frames, f) + timestamps = append(timestamps, entry.GetTimestamp().AsTime()) + bodies = append(bodies, body) + severities = append(severities, cloudlogging.GetLogLevel(entry.GetSeverity())) + ids = append(ids, id) + labels = append(labels, entryLabels) + traceIDs = append(traceIDs, traceID) } - // add the frames to the response. - for _, f := range frames { - response.Frames = append(response.Frames, f) + // Field order matters for Grafana's legacy logs parser, which falls back + // to the first time field and first string field. + frame := data.NewFrame(query.RefID, + data.NewField("timestamp", nil, timestamps), + data.NewField("body", nil, bodies), + data.NewField("severity", nil, severities), + data.NewField("id", nil, ids), + data.NewField("labels", nil, labels), + // Bare trace ID, nil when the entry has no trace. The frontend + // attaches the "View trace" data link to this field. + data.NewField("traceId", nil, traceIDs), + ) + frame.RefID = query.RefID + frame.Meta = &data.FrameMeta{ + Type: data.FrameTypeLogLines, + TypeVersion: data.FrameTypeVersion{0, 0}, + PreferredVisualization: data.VisTypeLogs, } + response.Frames = append(response.Frames, frame) return response } diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 5448d31..a715a1b 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -183,16 +183,159 @@ func TestQueryData_SingleLog(t *testing.T) { require.Len(t, resp.Responses[refID].Frames, 1) frame := resp.Responses[refID].Frames[0] - require.Equal(t, insertID, frame.Name) - require.Len(t, frame.Fields, 2) + requireLogLinesFrame(t, frame, refID) + require.Equal(t, 1, frame.Rows()) + require.Equal(t, time.UnixMilli(1660920349373).UTC(), frame.Fields[0].At(0).(time.Time).UTC()) + require.Equal(t, "Full log message from this GCE instance", frame.Fields[1].At(0)) + require.Equal(t, "info", frame.Fields[2].At(0)) + require.Equal(t, insertID, frame.Fields[3].At(0)) + require.JSONEq(t, `{ + "labels.\"custom_label\"": "custom_value", + "labels.\"instance_id\"": "unique", + "resource.type": "gce_instance", + "textPayload": "Full log message from this GCE instance", + "trace": "projects/xxx/traces/c0e331eab1515bbcd1b8306029902ff7" + }`, string(frame.Fields[4].At(0).(json.RawMessage))) + require.Equal(t, "c0e331eab1515bbcd1b8306029902ff7", *frame.Fields[5].At(0).(*string)) + + // The wire format must advertise the dataplane log-lines type so Grafana + // picks the dataplane parser rather than the legacy one. + serialized, err := frame.MarshalJSON() + require.NoError(t, err) + var wire struct { + Schema struct { + Name string `json:"name"` + RefID string `json:"refId"` + Meta struct { + Type string `json:"type"` + TypeVersion []int `json:"typeVersion"` + PreferredVisualization string `json:"preferredVisualisationType"` + } `json:"meta"` + Fields []struct { + Name string `json:"name"` + Type string `json:"type"` + } `json:"fields"` + } `json:"schema"` + } + require.NoError(t, json.Unmarshal(serialized, &wire)) + require.Equal(t, refID, wire.Schema.Name) + require.Equal(t, refID, wire.Schema.RefID) + require.Equal(t, "log-lines", wire.Schema.Meta.Type) + require.Equal(t, []int{0, 0}, wire.Schema.Meta.TypeVersion) + require.Equal(t, "logs", wire.Schema.Meta.PreferredVisualization) + var names, types []string + for _, f := range wire.Schema.Fields { + names = append(names, f.Name) + types = append(types, f.Type) + } + require.Equal(t, []string{"timestamp", "body", "severity", "id", "labels", "traceId"}, names) + require.Equal(t, []string{"time", "string", "string", "string", "other", "string"}, types) + client.AssertExpectations(t) +} + +// requireLogLinesFrame asserts the frame-level invariants of the dataplane +// logs contract: one frame per query, named and tagged with the refId. +func requireLogLinesFrame(t *testing.T, frame *data.Frame, refID string) { + t.Helper() + require.Equal(t, refID, frame.Name) + require.Equal(t, refID, frame.RefID) + require.Equal(t, data.FrameTypeLogLines, frame.Meta.Type) + require.Equal(t, data.FrameTypeVersion{0, 0}, frame.Meta.TypeVersion) require.Equal(t, data.VisTypeLogs, string(frame.Meta.PreferredVisualization)) + require.Len(t, frame.Fields, 6) + for i, name := range []string{"timestamp", "body", "severity", "id", "labels", "traceId"} { + require.Equal(t, name, frame.Fields[i].Name) + } +} - expectedFrame := []byte(`{"schema":{"name":"b6f39be2-b298-44da-9001-1f04e5756fa0","meta":{"typeVersion":[0,0],"preferredVisualisationType":"logs"},"fields":[{"name":"time","type":"time","typeInfo":{"frame":"time.Time"}},{"name":"content","type":"string","typeInfo":{"frame":"string"},"labels":{"id":"b6f39be2-b298-44da-9001-1f04e5756fa0","labels.\"custom_label\"":"custom_value","labels.\"instance_id\"":"unique","level":"info","resource.type":"gce_instance","textPayload":"Full log message from this GCE instance","trace":"projects/xxx/traces/c0e331eab1515bbcd1b8306029902ff7","traceId":"c0e331eab1515bbcd1b8306029902ff7"}}]},"data":{"values":[[1660920349373],["Full log message from this GCE instance"]]}}`) +// queryLogs runs a single query against a mocked client returning the given +// entries and returns the frames of the response. +func queryLogs(t *testing.T, entries []*loggingpb.LogEntry) (string, data.Frames) { + t.Helper() + to := time.Now() + from := to.Add(-1 * time.Hour) + client := mocks.NewAPI(t) + client.On("ListLogs", mock.Anything, mock.Anything).Return(entries, nil) + client.On("Close").Return(nil) - serializedFrame, err := frame.MarshalJSON() + ds := CloudLoggingDatasource{client: client} + refID := "logs" + resp, err := ds.QueryData(context.Background(), &backend.QueryDataRequest{ + Queries: []backend.DataQuery{ + { + JSON: []byte(`{"projectId": "testing", "queryText": "resource.type = \"testing\""}`), + RefID: refID, + TimeRange: backend.TimeRange{From: from, To: to}, + MaxDataPoints: 20, + }, + }, + }) + ds.Dispose() require.NoError(t, err) - require.Equal(t, string(expectedFrame), string(serializedFrame)) - client.AssertExpectations(t) + require.NoError(t, resp.Responses[refID].Error) + return refID, resp.Responses[refID].Frames +} + +func TestQueryData_MultipleLogs(t *testing.T) { + base := time.UnixMilli(1660920349373) + entries := []*loggingpb.LogEntry{ + { + InsertId: "insert-1", + Timestamp: timestamppb.New(base), + Severity: ltype.LogSeverity_ERROR, + Trace: "projects/proj-a/traces/aaaa", + Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "first"}, + }, + { + InsertId: "insert-2", + Timestamp: timestamppb.New(base.Add(-time.Second)), + Severity: ltype.LogSeverity_DEFAULT, + Payload: &loggingpb.LogEntry_TextPayload{TextPayload: "second"}, + }, + { + // No insert ID and no payload: must still yield a row with a + // unique id rather than being dropped or colliding. + Timestamp: timestamppb.New(base.Add(-2 * time.Second)), + Severity: ltype.LogSeverity_EMERGENCY, + }, + } + + refID, frames := queryLogs(t, entries) + require.Len(t, frames, 1, "all entries must land in a single frame") + frame := frames[0] + requireLogLinesFrame(t, frame, refID) + require.Equal(t, 3, frame.Rows()) + + var bodies, severities, ids []string + var traceIDs []*string + for i := 0; i < frame.Rows(); i++ { + bodies = append(bodies, frame.Fields[1].At(i).(string)) + severities = append(severities, frame.Fields[2].At(i).(string)) + ids = append(ids, frame.Fields[3].At(i).(string)) + traceIDs = append(traceIDs, frame.Fields[5].At(i).(*string)) + } + require.Equal(t, []string{"first", "second", ""}, bodies) + require.Equal(t, []string{"error", "info", "critical"}, severities) + require.Equal(t, []string{"insert-1", "insert-2", "logs_2"}, ids) + require.NotNil(t, traceIDs[0]) + require.Equal(t, "aaaa", *traceIDs[0]) + require.Nil(t, traceIDs[1]) + require.Nil(t, traceIDs[2]) + + // Order is preserved and timestamps are per row. + require.Equal(t, base.UTC(), frame.Fields[0].At(0).(time.Time).UTC()) + require.Equal(t, base.Add(-2*time.Second).UTC(), frame.Fields[0].At(2).(time.Time).UTC()) + + // Labels are per row and no longer duplicate the id/severity/traceId fields. + require.JSONEq(t, `{"trace": "projects/proj-a/traces/aaaa", "textPayload": "first"}`, string(frame.Fields[4].At(0).(json.RawMessage))) + require.JSONEq(t, `{}`, string(frame.Fields[4].At(2).(json.RawMessage))) +} + +func TestQueryData_EmptyLogs(t *testing.T) { + refID, frames := queryLogs(t, []*loggingpb.LogEntry{}) + require.Len(t, frames, 1, "an empty result still returns one frame so Grafana sees the schema") + requireLogLinesFrame(t, frames[0], refID) + require.Equal(t, 0, frames[0].Rows()) } func TestNewCloudLoggingDatasource_OAuthPassthrough(t *testing.T) { diff --git a/src/datasource.test.ts b/src/datasource.test.ts index da42f5e..10613d2 100644 --- a/src/datasource.test.ts +++ b/src/datasource.test.ts @@ -243,15 +243,37 @@ describe('Google Cloud Logging Data Source', () => { }); describe('logs to traces data links', () => { - const logFrame = (labels?: Labels): DataFrame => ({ - name: 'insert-id-1', + type Row = { traceId?: string | null; labels?: Labels }; + const BASE_FIELDS = 6; + + // Mirrors the backend's dataplane log-lines frame: one frame per + // query, one row per entry, nullable traceId, per-row labels. + const logFrame = (rows: Row[]): DataFrame => ({ + name: 'A', refId: 'A', - length: 1, + length: rows.length, fields: [ - { name: 'time', type: FieldType.time, config: {}, values: new ArrayVector([1700000000000]) }, - { name: 'content', type: FieldType.string, config: {}, labels, values: new ArrayVector(['hello']) }, + { name: 'timestamp', type: FieldType.time, config: {}, values: new ArrayVector(rows.map((_, i) => 1700000000000 + i)) }, + { name: 'body', type: FieldType.string, config: {}, values: new ArrayVector(rows.map((_, i) => `line ${i}`)) }, + { name: 'severity', type: FieldType.string, config: {}, values: new ArrayVector(rows.map(() => 'info')) }, + { name: 'id', type: FieldType.string, config: {}, values: new ArrayVector(rows.map((_, i) => `insert-id-${i}`)) }, + { name: 'labels', type: FieldType.other, config: {}, values: new ArrayVector(rows.map((r) => r.labels ?? {})) }, + { name: 'traceId', type: FieldType.string, config: {}, values: new ArrayVector(rows.map((r) => r.traceId ?? null)) }, ], }); + const tracedRow = (project = 'my-proj', traceId = 'abc123'): Row => ({ + traceId, + labels: { trace: `projects/${project}/traces/${traceId}`, spanId: 'def' }, + }); + + const field = (response: { data: any[] }, name: string) => + response.data[0].fields.find((f: { name: string }) => f.name === name); + const values = (f: any): unknown[] => (typeof f.values.toArray === 'function' ? f.values.toArray() : Array.from(f.values)); + const linkProjects = (response: { data: any[] }) => values(field(response, 'traceProject')); + const expectUntouched = (response: { data: any[] }) => { + expect(response.data[0].fields).toHaveLength(BASE_FIELDS); + expect(field(response, 'traceId').config.links).toBeUndefined(); + }; const runQuery = async (ds: DataSource, frame: DataFrame, targets: Query[] = []) => { jest.spyOn(DataSourceWithBackend.prototype, 'query').mockReturnValue(of({ data: [frame] })); @@ -262,14 +284,12 @@ describe('Google Cloud Logging Data Source', () => { jest.restoreAllMocks(); }); - it('appends a traceId field with an internal link when configured', async () => { + it('attaches an internal link to traceId and a hidden per-row traceProject field', async () => { const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); - const frame = logFrame({ trace: 'projects/my-proj/traces/abc123', traceId: 'abc123', spanId: 'def' }); - const response = await runQuery(ds, frame); + const response = await runQuery(ds, logFrame([tracedRow()])); - const traceField = response.data[0].fields.find((f: { name: string }) => f.name === 'traceId'); - expect(traceField).toBeDefined(); - expect(Array.from(traceField.values)).toEqual(['abc123']); + const traceField = field(response, 'traceId'); + expect(values(traceField)).toEqual(['abc123']); expect(traceField.config.links).toEqual([ { title: 'View trace', @@ -277,41 +297,56 @@ describe('Google Cloud Logging Data Source', () => { internal: { datasourceUid: 'trace-uid', datasourceName: 'Google Cloud Trace', - query: { refId: 'trace', queryType: 'traceID', traceId: 'abc123', projectId: 'my-proj' }, + query: { + refId: 'trace', + queryType: 'traceID', + traceId: '${__value.raw}', + projectId: '${__data.fields.traceProject}', + }, }, }, ]); + const projectField = field(response, 'traceProject'); + expect(projectField.type).toBe(FieldType.string); + expect(projectField.config.custom.hidden).toBe(true); + expect(values(projectField)).toEqual(['my-proj']); + }); + + it('resolves the project from each row\'s own trace path', async () => { + const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); + const response = await runQuery(ds, logFrame([tracedRow('proj-a', 't1'), tracedRow('proj-b', 't2')])); + expect(values(field(response, 'traceId'))).toEqual(['t1', 't2']); + expect(linkProjects(response)).toEqual(['proj-a', 'proj-b']); + }); + + it('leaves rows without a trace unlinked while linking the others', async () => { + const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); + const response = await runQuery(ds, logFrame([tracedRow(), { labels: { level: 'info' } }])); + expect(values(field(response, 'traceId'))).toEqual(['abc123', null]); + expect(linkProjects(response)).toEqual(['my-proj', null]); }); it('leaves frames untouched when logsToTraces is not configured', async () => { const ds = makeDataSource(); - const frame = logFrame({ trace: 'projects/my-proj/traces/abc123', traceId: 'abc123' }); - const response = await runQuery(ds, frame); - expect(response.data[0].fields).toHaveLength(2); + expectUntouched(await runQuery(ds, logFrame([tracedRow()]))); }); it('leaves frames untouched when the configured datasource does not resolve', async () => { const ds = makeDataSource({ logsToTraces: { datasourceUid: 'gone' } }); - const frame = logFrame({ trace: 'projects/my-proj/traces/abc123', traceId: 'abc123' }); - const response = await runQuery(ds, frame); - expect(response.data[0].fields).toHaveLength(2); + expectUntouched(await runQuery(ds, logFrame([tracedRow()]))); }); - it('leaves frames without a traceId label untouched', async () => { + it('leaves frames untouched when no row carries a trace', async () => { const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); - const frame = logFrame({ level: 'info' }); - const response = await runQuery(ds, frame); - expect(response.data[0].fields).toHaveLength(2); + expectUntouched(await runQuery(ds, logFrame([{ labels: { level: 'info' } }, {}]))); }); - it('removes the traceId label from the content field once the linked field is added', async () => { + it('leaves frames without a traceId field untouched', async () => { const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); - const frame = logFrame({ trace: 'projects/my-proj/traces/abc123', traceId: 'abc123' }); + const frame = logFrame([tracedRow()]); + frame.fields = frame.fields.filter((f) => f.name !== 'traceId'); const response = await runQuery(ds, frame); - - const contentField = response.data[0].fields.find((f: { name: string }) => f.name === 'content'); - expect(contentField.labels).toEqual({ trace: 'projects/my-proj/traces/abc123' }); - expect(response.data[0].fields.find((f: { name: string }) => f.name === 'traceId')).toBeDefined(); + expect(response.data[0].fields).toHaveLength(BASE_FIELDS - 1); }); it('falls back to the default project when the trace label is not a resource path', async () => { @@ -319,28 +354,27 @@ describe('Google Cloud Logging Data Source', () => { logsToTraces: { datasourceUid: 'trace-uid' }, defaultProject: 'my-default-proj', }); - const frame = logFrame({ trace: 'abc123', traceId: 'abc123' }); - const response = await runQuery(ds, frame); + const response = await runQuery(ds, logFrame([{ traceId: 'abc123', labels: { trace: 'abc123' } }])); + expect(linkProjects(response)).toEqual(['my-default-proj']); + }); - const traceField = response.data[0].fields.find((f: { name: string }) => f.name === 'traceId'); - expect(traceField.config.links[0].internal.query.projectId).toBe('my-default-proj'); + it('skips the link when no project can be determined for any row', async () => { + const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); + expectUntouched(await runQuery(ds, logFrame([{ traceId: 'abc123', labels: { trace: 'abc123' } }]))); }); - it('skips the link when no project can be determined', async () => { + it('nulls out the trace id of rows whose project cannot be resolved so they get no broken link', async () => { const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); - const frame = logFrame({ trace: 'abc123', traceId: 'abc123' }); - const response = await runQuery(ds, frame); - expect(response.data[0].fields).toHaveLength(2); + const response = await runQuery(ds, logFrame([tracedRow(), { traceId: 'zzz', labels: { trace: 'zzz' } }])); + expect(values(field(response, 'traceId'))).toEqual(['abc123', null]); + expect(linkProjects(response)).toEqual(['my-proj', null]); }); it('keeps the trace-path project when targets carry a projectId and the flag is off', async () => { const ds = makeDataSource({ logsToTraces: { datasourceUid: 'trace-uid' } }); - const frame = logFrame({ trace: 'projects/my-proj/traces/abc123', traceId: 'abc123' }); const targets = [{ refId: 'A', projectId: 'other-proj' } as Query]; - const response = await runQuery(ds, frame, targets); - - const traceField = response.data[0].fields.find((f: { name: string }) => f.name === 'traceId'); - expect(traceField.config.links[0].internal.query.projectId).toBe('my-proj'); + const response = await runQuery(ds, logFrame([tracedRow()]), targets); + expect(linkProjects(response)).toEqual(['my-proj']); }); it('warms the GCE default project for the trace-link fallback when the flag is off', async () => { @@ -352,13 +386,11 @@ describe('Google Cloud Logging Data Source', () => { // The target carries a projectId, so the warm-up is not needed to // build the request — only the non-canonical trace-path fallback // consumes it when the response is mapped. - const frame = logFrame({ trace: 'abc123', traceId: 'abc123' }); const targets = [{ refId: 'A', projectId: 'some-proj' } as Query]; - const response = await runQuery(ds, frame, targets); + const response = await runQuery(ds, logFrame([{ traceId: 'abc123', labels: { trace: 'abc123' } }]), targets); expect(gceSpy).toHaveBeenCalled(); - const traceField = response.data[0].fields.find((f: { name: string }) => f.name === 'traceId'); - expect(traceField.config.links[0].internal.query.projectId).toBe('gce-proj'); + expect(linkProjects(response)).toEqual(['gce-proj']); }); describe('with projectIdFromQuery enabled', () => { @@ -372,51 +404,52 @@ describe('Google Cloud Logging Data Source', () => { stubTemplateSrv ); - const linkProject = (response: { data: any[] }) => - response.data[0].fields.find((f: { name: string }) => f.name === 'traceId')?.config.links[0].internal - .query.projectId; - - const routedFrame = () => - logFrame({ trace: 'projects/routing-proj/traces/abc123', traceId: 'abc123' }); + const routedFrame = () => logFrame([tracedRow('routing-proj')]); it('uses the projectId of the target that produced the frame, not the trace path', async () => { const ds = makeFlagOnDataSource(); const targets = [{ refId: 'A', projectId: 'tenant-proj' } as Query]; const response = await runQuery(ds, routedFrame(), targets); - expect(linkProject(response)).toBe('tenant-proj'); + expect(linkProjects(response)).toEqual(['tenant-proj']); + }); + + it('applies the target project to every row of the frame', async () => { + const ds = makeFlagOnDataSource(); + const targets = [{ refId: 'A', projectId: 'tenant-proj' } as Query]; + const response = await runQuery(ds, logFrame([tracedRow('routing-a', 't1'), tracedRow('routing-b', 't2')]), targets); + expect(linkProjects(response)).toEqual(['tenant-proj', 'tenant-proj']); }); it('interpolates template variables in the target projectId', async () => { const ds = makeFlagOnDataSource(); const targets = [{ refId: 'A', projectId: '$project' } as Query]; const response = await runQuery(ds, routedFrame(), targets); - expect(linkProject(response)).toBe('tenant-proj'); + expect(linkProjects(response)).toEqual(['tenant-proj']); }); it('falls back to the default project when no target matches the frame', async () => { const ds = makeFlagOnDataSource({ defaultProject: 'my-default-proj' }); const response = await runQuery(ds, routedFrame(), []); - expect(linkProject(response)).toBe('my-default-proj'); + expect(linkProjects(response)).toEqual(['my-default-proj']); }); it('falls back to the default project when the matching target has no projectId', async () => { const ds = makeFlagOnDataSource({ defaultProject: 'my-default-proj' }); const targets = [{ refId: 'A', projectId: '' } as Query]; const response = await runQuery(ds, routedFrame(), targets); - expect(linkProject(response)).toBe('my-default-proj'); + expect(linkProjects(response)).toEqual(['my-default-proj']); }); it('ignores hidden targets when resolving the project', async () => { const ds = makeFlagOnDataSource({ defaultProject: 'my-default-proj' }); const targets = [{ refId: 'A', projectId: 'tenant-proj', hide: true } as Query]; const response = await runQuery(ds, routedFrame(), targets); - expect(linkProject(response)).toBe('my-default-proj'); + expect(linkProjects(response)).toEqual(['my-default-proj']); }); it('omits the link when neither a target project nor a default project resolves', async () => { const ds = makeFlagOnDataSource(); - const response = await runQuery(ds, routedFrame(), []); - expect(response.data[0].fields).toHaveLength(2); + expectUntouched(await runQuery(ds, routedFrame(), [])); }); it('pre-resolves the GCE default project so the link fallback works under GCE auth', async () => { @@ -427,7 +460,7 @@ describe('Google Cloud Logging Data Source', () => { const targets = [{ refId: 'B', projectId: 'other-proj' } as Query]; const response = await runQuery(ds, routedFrame(), targets); expect(gceSpy).toHaveBeenCalled(); - expect(linkProject(response)).toBe('gce-proj'); + expect(linkProjects(response)).toEqual(['gce-proj']); }); }); }); diff --git a/src/datasource.ts b/src/datasource.ts index 80eda29..51469fa 100644 --- a/src/datasource.ts +++ b/src/datasource.ts @@ -341,12 +341,18 @@ export class DataSource extends DataSourceWithBackend/traces/` path, `traceId` the bare ID). Surface - * the trace ID as its own field carrying an internal data link, so the - * log details panel renders a "View trace" link that opens the configured - * tracing data source — the same mechanism as Loki's derived fields. + * The backend emits one dataplane log-lines frame per query. Each row has a + * nullable `traceId` field (the bare trace ID, null when the entry has no + * trace) and a `labels` object whose `trace` entry is the raw LogEntry value + * in the canonical `projects//traces/` form. Attach an internal + * data link to `traceId` so the log details panel renders a "View trace" + * link that opens the configured tracing data source — the same mechanism + * as Loki's derived fields. + * + * The link's project can differ per row, and a data link is configured per + * field, so the per-row project is carried in a hidden `traceProject` field + * and interpolated into the link via `${__data.fields.traceProject}`; the + * trace ID itself is `${__value.raw}`. * * `projectIdOverride` is defined when the projectIdFromQuery setting is * on: it is used verbatim as the link's project and the trace path is @@ -360,46 +366,67 @@ export class DataSource extends DataSourceWithBackend f.name === 'content'); - const labels = contentField?.labels; - const traceId = labels?.['traceId']; - if (!contentField || !labels || !traceId || frame.fields.some((f) => f.name === 'traceId')) { + const traceField = frame.fields.find((f) => f.name === 'traceId'); + if (!traceField || frame.fields.some((f) => f.name === 'traceProject')) { + return frame; + } + const traceIds = fieldValues(traceField); + if (!traceIds.some((id) => !!id)) { return frame; } + const labelsField = frame.fields.find((f) => f.name === 'labels'); + const labelRows = labelsField ? fieldValues | null | undefined>(labelsField) : []; + // LogEntry.trace is a free-form string; when it isn't the canonical // resource path, fall back to the default project, and if that is also - // unset skip the link entirely — Cloud Trace errors on an empty project, - // so no link beats a broken one. - const projectId = - projectIdOverride !== undefined - ? projectIdOverride - : labels['trace']?.match(/^projects\/([^/]+)\/traces\//)?.[1] ?? this.defaultProjectSync(); - if (!projectId) { + // unset skip the link for that row — Cloud Trace errors on an empty + // project, so no link beats a broken one. + const projects = traceIds.map((traceId, i) => { + if (!traceId) { + return null; + } + const projectId = + projectIdOverride !== undefined + ? projectIdOverride + : labelRows[i]?.['trace']?.match(/^projects\/([^/]+)\/traces\//)?.[1] ?? this.defaultProjectSync(); + return projectId || null; + }); + if (!projects.some((p) => !!p)) { return frame; } - // The linked field replaces the label in the log details view; leaving - // both would show `traceId` twice (once without the link). - delete labels['traceId']; - const rowCount = contentField.values.length; - frame.fields.push({ - name: 'traceId', - type: FieldType.string, - config: { - links: [ - { - title: 'View trace', - url: '', - internal: { - datasourceUid, - datasourceName, - query: { refId: 'trace', queryType: 'traceID', traceId, projectId }, + + // A data link applies to every non-null value of its field, so rows whose + // project could not be resolved get their trace ID nulled out rather + // than a broken link. The full trace path stays visible in `labels`. + traceField.values = projects.map((p, i) => (p ? traceIds[i] : null)) as unknown as typeof traceField.values; + traceField.config = { + ...(traceField.config ?? {}), + links: [ + { + title: 'View trace', + url: '', + internal: { + datasourceUid, + datasourceName, + query: { + refId: 'trace', + queryType: 'traceID', + traceId: '${__value.raw}', + projectId: '${__data.fields.traceProject}', }, }, - ], - }, + }, + ], + }; + frame.fields.push({ + name: 'traceProject', + type: FieldType.string, + // `custom.hidden` is what Grafana's log details parser checks to keep a + // field out of the details view while leaving it available to links. + config: { custom: { hidden: true } }, // Grafana >= 10 accepts plain arrays as field values at runtime; the // cast only satisfies the bundled @grafana/data 9.x typings (Vector). - values: new Array(rowCount).fill(traceId), + values: projects, } as unknown as Field); return frame; } @@ -430,14 +457,8 @@ export class DataSource extends DataSourceWithBackend= 10 hands us plain arrays; the + * bundled @grafana/data 9.x typings (and tests) still model them as Vector. + */ +function fieldValues(field: Field): T[] { + const v = field.values as unknown; + if (Array.isArray(v)) { + return v as T[]; + } + const vec = v as { toArray?: () => T[] }; + return typeof vec?.toArray === 'function' ? vec.toArray() : Array.from(v as Iterable); +} + +/** + * Maps a Grafana log level (as shown in the logs panel) back to the Cloud + * Logging severity it was derived from. Grafana has no DEFAULT or EMERGENCY. + */ +function toCloudLoggingSeverity(level: string): string { + if (level === 'debug') { + return 'DEFAULT'; + } + if (level === 'critical') { + return 'EMERGENCY'; + } + return level; +} From 8f5377aa6552066fb4c9e5cf20362d6d1b20eedb Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 3 Sep 2026 21:45:27 +0000 Subject: [PATCH 3/5] fix for #212 --- CHANGELOG.md | 2 + pkg/plugin/cloudlogging/client.go | 18 +- pkg/plugin/cloudlogging/cloudlogging_test.go | 27 ++ src/CloudLoggingVariableFindQuery.test.ts | 123 +++++++ src/CloudLoggingVariableFindQuery.ts | 107 +++--- src/Fields.tsx | 6 + src/VariableQueryEditor.test.tsx | 211 +++++++++++ src/VariableQueryEditor.tsx | 363 +++++++++++-------- src/types.ts | 4 +- 9 files changed, 644 insertions(+), 217 deletions(-) create mode 100644 src/CloudLoggingVariableFindQuery.test.ts create mode 100644 src/VariableQueryEditor.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c190ab..7460cf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ * **Breaking: logs responses now use Grafana's dataplane `log-lines` format** ([#221](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/221)). Each query returns a single frame with one row per log entry instead of one frame per entry. Fields are `timestamp`, `body`, `severity`, `id` (the entry's insert ID), `labels` (per-row JSON object) and `traceId`; previously they were `time` and `content` with the metadata attached as labels on `content`. Grafana can now identify each log line uniquely, which fixes log details expanding every line at once, the log list jumping to the top on click, broken permalinks and dedup, and the Logs Table showing only the first line. Dashboards that reference the old `content` or `time` field names in transformations or Table panels need updating * "View trace" links are resolved per log line. The project comes from each entry's own trace path (or the query/default project, as before), so a single result set spanning several projects links each line to the right trace * An empty result now returns an empty frame with the logs schema instead of no frame +* Fix the query variable editor getting stuck on "Loading..." ([#212](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/212)). The editor no longer fetches buckets for an empty project or lists projects before they are needed; option lists load per scope, failures are shown inline instead of freezing the editor, and variable query errors (for example a disabled Cloud Resource Manager API) now surface in Grafana instead of silently producing no values. Selecting a scope is saved immediately +* Fix an empty query text producing an unparseable filter (a leading `AND`); the time range alone is sent instead ## 1.7.2 (2026-08-17) * Update dependencies to address security vulnerabilities flagged by the Grafana plugin review: js-yaml (CVE-2026-59869) and nanoid (CVE-2026-67213) in the frontend build toolchain diff --git a/pkg/plugin/cloudlogging/client.go b/pkg/plugin/cloudlogging/client.go index acb6744..11cac2b 100644 --- a/pkg/plugin/cloudlogging/client.go +++ b/pkg/plugin/cloudlogging/client.go @@ -203,11 +203,21 @@ type Query struct { } // String is the query formatted for querying GCP -// It is the query text, with the time range constraints appended +// It is the query text, with the time range constraints appended. +// +// No parentheses are needed around the user's filter: in the Logging query +// language OR binds tighter than AND, so `a OR b AND timestamp >= x` already +// groups as `(a OR b) AND timestamp >= x`. +// +// An empty filter yields just the time range; a leading `AND` is rejected by +// the API as an unparseable filter. func (q *Query) String() string { - return fmt.Sprintf(`%s AND timestamp >= "%s" AND timestamp <= "%s"`, - q.Filter, q.TimeRange.From, q.TimeRange.To, - ) + timeRange := fmt.Sprintf(`timestamp >= "%s" AND timestamp <= "%s"`, q.TimeRange.From, q.TimeRange.To) + filter := strings.TrimSpace(q.Filter) + if filter == "" { + return timeRange + } + return filter + " AND " + timeRange } // ListProjects returns the project IDs of all visible projects. diff --git a/pkg/plugin/cloudlogging/cloudlogging_test.go b/pkg/plugin/cloudlogging/cloudlogging_test.go index 342b3b7..8f2a723 100644 --- a/pkg/plugin/cloudlogging/cloudlogging_test.go +++ b/pkg/plugin/cloudlogging/cloudlogging_test.go @@ -456,3 +456,30 @@ func normalizeLabelSpaces(labels data.Labels) data.Labels { } return normalized } + +func TestQueryString(t *testing.T) { + timeRange := struct { + From string + To string + }{From: "2026-01-01T00:00:00Z", To: "2026-01-02T00:00:00Z"} + suffix := `timestamp >= "2026-01-01T00:00:00Z" AND timestamp <= "2026-01-02T00:00:00Z"` + + testCases := []struct { + name string + filter string + expected string + }{ + {name: "simple filter", filter: `severity >= DEFAULT`, expected: `severity >= DEFAULT AND ` + suffix}, + {name: "top-level OR is left as is (OR binds tighter than AND)", filter: `a="1" OR b="2"`, expected: `a="1" OR b="2" AND ` + suffix}, + {name: "multi-line filter keeps inner newlines", filter: "a=\"1\"\nb=\"2\"", expected: "a=\"1\"\nb=\"2\" AND " + suffix}, + {name: "surrounding whitespace is trimmed", filter: " severity >= DEFAULT \n", expected: `severity >= DEFAULT AND ` + suffix}, + {name: "empty filter yields only the time range", filter: "", expected: suffix}, + {name: "whitespace-only filter yields only the time range", filter: " \n\t", expected: suffix}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + q := &cloudlogging.Query{Filter: tc.filter, TimeRange: timeRange} + require.Equal(t, tc.expected, q.String()) + }) + } +} diff --git a/src/CloudLoggingVariableFindQuery.test.ts b/src/CloudLoggingVariableFindQuery.test.ts new file mode 100644 index 0000000..18aadf6 --- /dev/null +++ b/src/CloudLoggingVariableFindQuery.test.ts @@ -0,0 +1,123 @@ +/** + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import CloudLoggingVariableFindQuery from './CloudLoggingVariableFindQuery'; +import { DataSource } from './datasource'; +import { CloudLoggingVariableQuery, LogFindQueryScopes } from './types'; + +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getTemplateSrv: () => ({ + replace: (s?: string) => (s === '$project' ? 'tenant-proj' : s === '$bucket' ? 'global/buckets/_Default' : s === '$empty' ? '' : s ?? ''), + }), +})); + +const makeDataSource = (overrides: Record = {}) => ({ + getDefaultProject: jest.fn().mockResolvedValue('default-proj'), + getFilteredProjects: jest.fn().mockResolvedValue(['proj-a', 'proj-b']), + getFilteredBuckets: jest.fn().mockResolvedValue(['global/buckets/_Default', 'global/buckets/app']), + getLogBucketViews: jest.fn().mockResolvedValue(['_AllLogs', 'errors']), + ...overrides, +}); + +const query = (q: Partial) => ({ refId: 'v', projectId: '', ...q } as CloudLoggingVariableQuery); + +describe('CloudLoggingVariableFindQuery', () => { + it('lists projects', async () => { + const ds = makeDataSource(); + const result = await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute( + query({ selectedQueryType: LogFindQueryScopes.Projects }) + ); + expect(result).toEqual([ + { text: 'proj-a', value: 'proj-a', expandable: true }, + { text: 'proj-b', value: 'proj-b', expandable: true }, + ]); + }); + + it('lists buckets of the default project when the query has none', async () => { + const ds = makeDataSource(); + const result = await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute( + query({ selectedQueryType: LogFindQueryScopes.Buckets }) + ); + expect(ds.getFilteredBuckets).toHaveBeenCalledWith('default-proj'); + expect(result.map((r) => r.value)).toEqual(['global/buckets/_Default', 'global/buckets/app']); + }); + + it('interpolates a template variable project', async () => { + const ds = makeDataSource(); + await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute( + query({ selectedQueryType: LogFindQueryScopes.Buckets, projectId: '$project' }) + ); + expect(ds.getFilteredBuckets).toHaveBeenCalledWith('tenant-proj'); + }); + + it('fails with an actionable message instead of sending an empty project', async () => { + const ds = makeDataSource({ getDefaultProject: jest.fn().mockResolvedValue('') }); + await expect( + new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute(query({ selectedQueryType: LogFindQueryScopes.Buckets })) + ).rejects.toThrow(/Cannot list log buckets: select a project in the variable query or configure a default project/); + expect(ds.getFilteredBuckets).not.toHaveBeenCalled(); + }); + + it('treats a template variable that resolves to nothing as no project', async () => { + const ds = makeDataSource(); + await expect( + new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute( + query({ selectedQueryType: LogFindQueryScopes.Views, projectId: '$empty', bucketId: 'global/buckets/app' }) + ) + ).rejects.toThrow(/Cannot list log views/); + }); + + it('lists views for a project and bucket, interpolating both', async () => { + const ds = makeDataSource(); + const result = await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute( + query({ selectedQueryType: LogFindQueryScopes.Views, projectId: '$project', bucketId: '$bucket' }) + ); + expect(ds.getLogBucketViews).toHaveBeenCalledWith('tenant-proj', 'global/buckets/_Default'); + expect(result.map((r) => r.value)).toEqual(['_AllLogs', 'errors']); + }); + + it('returns no views when no bucket is selected', async () => { + const ds = makeDataSource(); + const result = await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute( + query({ selectedQueryType: LogFindQueryScopes.Views, projectId: 'proj-a' }) + ); + expect(result).toEqual([]); + expect(ds.getLogBucketViews).not.toHaveBeenCalled(); + }); + + it('returns nothing for an unknown scope', async () => { + const ds = makeDataSource(); + const result = await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute(query({ selectedQueryType: 'nope' })); + expect(result).toEqual([]); + }); + + it('propagates backend errors instead of returning an empty list', async () => { + const ds = makeDataSource({ + getFilteredProjects: jest.fn().mockRejectedValue({ status: 502, data: { message: 'Cloud Resource Manager API has not been used' } }), + }); + await expect( + new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute(query({ selectedQueryType: LogFindQueryScopes.Projects })) + ).rejects.toMatchObject({ status: 502 }); + }); + + it('does not mutate the saved query model', async () => { + const ds = makeDataSource(); + const q = query({ selectedQueryType: LogFindQueryScopes.Buckets }); + await new CloudLoggingVariableFindQuery(ds as unknown as DataSource).execute(q); + expect(q.projectId).toBe(''); + }); +}); diff --git a/src/CloudLoggingVariableFindQuery.ts b/src/CloudLoggingVariableFindQuery.ts index 606f960..9ba4cc7 100644 --- a/src/CloudLoggingVariableFindQuery.ts +++ b/src/CloudLoggingVariableFindQuery.ts @@ -15,79 +15,76 @@ */ import { SelectableValue } from '@grafana/data'; +import { getTemplateSrv } from '@grafana/runtime'; import { DataSource } from './datasource'; import { CloudLoggingVariableQuery, LogFindQueryScopes } from './types'; -import { getTemplateSrv } from '@grafana/runtime'; +const toOption = (value: string): SelectableValue => ({ text: value, value, expandable: true }); + +/** + * Resolves the values of a Cloud Logging query variable. + * + * Errors are deliberately not swallowed: Grafana shows them in the variable + * editor and dashboard settings, which beats silently offering no values + * (a disabled Cloud Resource Manager API, for example, used to look like an + * empty project list). + */ export default class CloudLoggingVariableFindQuery { constructor(private datasource: DataSource) { } - async execute(query: CloudLoggingVariableQuery) { - try { - if (!query.projectId) { - query.projectId = await this.datasource.getDefaultProject(); - } - switch (query.selectedQueryType) { - case LogFindQueryScopes.Projects: - return this.handleProjectsQuery(); - case LogFindQueryScopes.Buckets: - return this.handleBucketQuery(query) - case LogFindQueryScopes.Views: - return this.handleViewQuery(query) - default: - return []; - } - } catch (error) { - console.error(`Could not run CloudLoggingVariableFindQuery ${query}`, error); - return []; + async execute(query: CloudLoggingVariableQuery): Promise>> { + const projectId = query.projectId || (await this.datasource.getDefaultProject()); + switch (query.selectedQueryType) { + case LogFindQueryScopes.Projects: + return this.handleProjectsQuery(); + case LogFindQueryScopes.Buckets: + return this.handleBucketQuery(projectId); + case LogFindQueryScopes.Views: + return this.handleViewQuery(projectId, query.bucketId); + default: + return []; } } async handleProjectsQuery() { const projects = await this.datasource.getFilteredProjects(); - return (projects).map((s) => ({ - text: s, - value: s, - expandable: true, - } as SelectableValue)); + return projects.map(toOption); } - async handleBucketQuery({ projectId }: CloudLoggingVariableQuery) { - let buckets: string[] = []; - let p = projectId - if (projectId.startsWith('$')) { - p = getTemplateSrv().replace(projectId) - } - buckets = await this.datasource.getFilteredBuckets(p); - return (buckets).map((s) => ({ - text: s, - value: s, - expandable: true, - } as SelectableValue)); + async handleBucketQuery(projectId: string) { + const buckets = await this.datasource.getFilteredBuckets(this.resolveProject(projectId, 'log buckets')); + return buckets.map(toOption); } - async handleViewQuery({ projectId, bucketId }: CloudLoggingVariableQuery) { + async handleViewQuery(projectId: string, bucketId?: string) { if (!bucketId) { - return [] - } - let views: string[] = []; - let p = projectId - if (projectId.startsWith('$')) { - p = getTemplateSrv().replace(projectId) - } - let b = bucketId - if (bucketId.startsWith('$')) { - b = getTemplateSrv().replace(bucketId) + return []; } + const bucket = this.interpolate(bucketId); // Return if we don't know the bucket - if (!b) { - return [] + if (!bucket) { + return []; + } + const views = await this.datasource.getLogBucketViews(this.resolveProject(projectId, 'log views'), bucket); + return views.map(toOption); + } + + /** Interpolates a `$variable` reference; other values pass through. */ + private interpolate(value: string): string { + return value.startsWith('$') ? getTemplateSrv().replace(value) : value; + } + + /** + * The backend rejects an empty project, so fail with a message that says + * what to do rather than "Missing required parameter: ProjectId". + */ + private resolveProject(projectId: string, what: string): string { + const project = this.interpolate(projectId); + if (!project) { + throw new Error( + `Cannot list ${what}: select a project in the variable query or configure a default project on the data source.` + ); } - views = await this.datasource.getLogBucketViews(p, b); - return (views).map((s) => ({ - text: s, - value: s, - expandable: true, - } as SelectableValue)); + return project; } } diff --git a/src/Fields.tsx b/src/Fields.tsx index 2a754e3..e380e32 100644 --- a/src/Fields.tsx +++ b/src/Fields.tsx @@ -25,6 +25,8 @@ interface VariableQueryFieldProps { value: string; label: string; allowCustomValue?: boolean; + isLoading?: boolean; + inputId?: string; } export const VariableQueryField = ({ @@ -33,12 +35,16 @@ export const VariableQueryField = ({ value, options, allowCustomValue = false, + isLoading = false, + inputId, }: VariableQueryFieldProps) => { return ( - - - - - ); - } - - return ( - <> - this.onQueryTypeChange(value)} - label="Logging Scope" - /> - {this.renderLogScopeSwitch(this.state.selectedQueryType)} - - ); + } + + private async loadBuckets(projectId: string) { + // An empty project used to be sent to the backend, which rejected it and + // left the editor stuck on "Loading..." (#212). + if (!projectId || isTemplateVariable(projectId)) { + this.setState({ buckets: [] }); + return; + } + this.setState({ loading: true }); + try { + const buckets = await this.props.datasource.getFilteredBuckets(projectId); + if (!this.unmounted) { + this.setState({ buckets: buckets.map(toOption) }); + } + } catch (err) { + if (!this.unmounted) { + this.setState({ buckets: [], error: `Could not load log buckets for ${projectId}: ${describeError(err)}` }); + } + } finally { + if (!this.unmounted) { + this.setState({ loading: false }); + } } + } + + render() { + const { selectedQueryType, projectId, bucketId, projects, buckets, loading, error } = this.state; + const variableOptionGroup = { + label: 'Template Variables', + expanded: false, + options: getTemplateSrv() + .getVariables() + .map((v) => toOption(`$${v.name}`)), + }; + + return ( + <> + + {selectedQueryType !== LogFindQueryScopes.Projects && ( + + )} + {selectedQueryType === LogFindQueryScopes.Views && ( + + )} + {error && } + + ); + } } diff --git a/src/types.ts b/src/types.ts index a490c0d..b1bd864 100644 --- a/src/types.ts +++ b/src/types.ts @@ -110,7 +110,9 @@ export interface VariableScopeData { projects: SelectableValue[]; buckets: SelectableValue[]; bucketId: string; - viewId: string; projectId: string; + /** True while an option list for the current scope is being fetched. */ loading: boolean; + /** Last option-loading failure, shown inline; cleared on the next user action. */ + error?: string; } From 2782c288b0a802fd2d5cdf686c265475e903faa5 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 3 Sep 2026 21:57:54 +0000 Subject: [PATCH 4/5] fix issue #202 --- CHANGELOG.md | 1 + README.md | 22 ++++++++++ pkg/plugin/plugin.go | 27 ++++++++----- pkg/plugin/plugin_test.go | 84 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 124 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7460cf5..7b5afeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * An empty result now returns an empty frame with the logs schema instead of no frame * Fix the query variable editor getting stuck on "Loading..." ([#212](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/212)). The editor no longer fetches buckets for an empty project or lists projects before they are needed; option lists load per scope, failures are shown inline instead of freezing the editor, and variable query errors (for example a disabled Cloud Resource Manager API) now surface in Grafana instead of silently producing no values. Selecting a scope is saved immediately * Fix an empty query text producing an unparseable filter (a leading `AND`); the time range alone is sent instead +* Fix JWT authentication failing with "An error occurred within the plugin" when the data source is provisioned (YAML, Terraform, environment variables) and the private key contains literal `\n` escape sequences instead of line breaks ([#76](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/76), [#202](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/202)). The key is now read the same way as in the Google Cloud Monitoring data source, which also adds support for `privateKeyPath`. A key that still fails to parse produces an explanatory error instead of a generic one ## 1.7.2 (2026-08-17) * Update dependencies to address security vulnerabilities flagged by the Grafana plugin review: js-yaml (CVE-2026-59869) and nanoid (CVE-2026-67213) in the frontend build toolchain diff --git a/README.md b/README.md index dab96c8..9a5bee9 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,28 @@ datasources: # universeDomain: googleapis.com ``` +To provision a service account key (JWT authentication) instead, supply the fields from the service account JSON file. The private key goes in `secureJsonData`, either inline or via a file path: + +```yaml +apiVersion: 1 + +datasources: + - name: Google Cloud Logging + type: googlecloud-logging-datasource + access: proxy + jsonData: + authenticationType: jwt + clientEmail: my-service-account@my-project.iam.gserviceaccount.com + defaultProject: my-project + tokenUri: https://oauth2.googleapis.com/token + # Alternative to secureJsonData.privateKey: read the PEM file from disk + # privateKeyPath: /etc/secrets/gcp-logging-private-key.pem + secureJsonData: + privateKey: $__file{/etc/secrets/gcp-logging-private-key.pem} +``` + +The same `jsonData` and `secureJsonData` fields work with the Grafana Terraform provider's `grafana_data_source` resource. The `privateKey` value is the `private_key` field of the service account JSON file. It may be passed with real line breaks or with the literal `\n` escape sequences as they appear in the JSON file; both are accepted. + ### Supported variables The plugin currently supports variables for logging scopes. For example, you can define a project variable and switch between projects. The following screenshot shows an example using project, bucket, and view. diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index aa8d6cd..5e5b80c 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -43,7 +43,6 @@ var ( ) const ( - privateKeyKey = "privateKey" gceAuthentication = "gce" jwtAuthentication = "jwt" accessTokenAuthentication = "accessToken" @@ -97,16 +96,23 @@ func NewCloudLoggingDatasource(ctx context.Context, settings backend.DataSourceI conf.AuthType = jwtAuthentication } + // Read the private key the same way the other Google data sources do + // (grafana-google-sdk-go): from `privateKeyPath` if set, else from the + // `privateKey` secret, with literal `\n` sequences turned into newlines. + // Provisioned keys (YAML, Terraform, env vars) often arrive with the + // escapes intact, which the credentials parser rejects (#76, #202). + privateKey, err := utils.GetPrivateKey(&settings) + if err != nil { + return nil, fmt.Errorf("read private key: %s", sanitizeErrorMessage(err)) + } + // Only auto-switch to accessToken if the auth type is jwt (the default) and // no JWT private key was provided. This preserves backward compat for // pre-dropdown users (v1.5.0) who only set an access token, without hijacking // explicitly-chosen auth types like GCE or OAuth. - if conf.AuthType == jwtAuthentication { + if conf.AuthType == jwtAuthentication && privateKey == "" { if accessToken, ok := settings.DecryptedSecureJSONData[accessTokenKey]; ok && accessToken != "" { - privateKey, hasKey := settings.DecryptedSecureJSONData[privateKeyKey] - if !hasKey || privateKey == "" { - conf.AuthType = accessTokenAuthentication - } + conf.AuthType = accessTokenAuthentication } } @@ -117,8 +123,7 @@ func NewCloudLoggingDatasource(ctx context.Context, settings backend.DataSourceI switch conf.AuthType { case jwtAuthentication: - privateKey, ok := settings.DecryptedSecureJSONData[privateKeyKey] - if !ok || privateKey == "" { + if privateKey == "" { return nil, errMissingCredentials } @@ -151,7 +156,11 @@ func NewCloudLoggingDatasource(ctx context.Context, settings backend.DataSourceI } if client_err != nil { - return nil, fmt.Errorf("create client: %s", sanitizeErrorMessage(client_err)) + msg := sanitizeErrorMessage(client_err) + if conf.AuthType == jwtAuthentication && strings.Contains(msg, "parse key") { + msg += " (the privateKey must be the complete PEM block from the service account JSON file, including its line breaks)" + } + return nil, fmt.Errorf("create client: %s", msg) } return &CloudLoggingDatasource{ diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index a715a1b..8ce2f20 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -16,8 +16,15 @@ package plugin import ( "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" "encoding/json" + "encoding/pem" "errors" + "os" + "path/filepath" + "strings" "testing" "time" @@ -458,7 +465,7 @@ func TestNewCloudLoggingDatasource_AuthOverride(t *testing.T) { settings := backend.DataSourceInstanceSettings{ JSONData: []byte(jsonData), DecryptedSecureJSONData: map[string]string{ - privateKeyKey: "dummy-private-key", + "privateKey": "dummy-private-key", accessTokenKey: "dummy-access-token", }, } @@ -507,6 +514,81 @@ func TestNewCloudLoggingDatasource_AuthOverride(t *testing.T) { }) } +// testPrivateKeyPEM returns a freshly generated PKCS#8 private key in PEM +// form, i.e. what the `private_key` field of a service account JSON file +// contains once its JSON escapes are decoded. +func testPrivateKeyPEM(t *testing.T) string { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + der, err := x509.MarshalPKCS8PrivateKey(key) + require.NoError(t, err) + return string(pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der})) +} + +const jwtJSONData = `{"authenticationType": "jwt", "clientEmail": "sa@test-project.iam.gserviceaccount.com", "defaultProject": "test-project", "tokenUri": "https://oauth2.googleapis.com/token"}` + +func TestNewCloudLoggingDatasource_JWTPrivateKey(t *testing.T) { + pemKey := testPrivateKeyPEM(t) + + t.Run("key with real line breaks", func(t *testing.T) { + inst, err := NewCloudLoggingDatasource(context.Background(), backend.DataSourceInstanceSettings{ + JSONData: []byte(jwtJSONData), + DecryptedSecureJSONData: map[string]string{"privateKey": pemKey}, + }) + require.NoError(t, err) + require.NotNil(t, inst.(*CloudLoggingDatasource).client) + }) + + // Regression for #76 / #202: keys provisioned via YAML, Terraform or env + // vars often keep the JSON file's literal `\n` escapes. + t.Run("key with literal backslash-n escapes", func(t *testing.T) { + escaped := strings.ReplaceAll(pemKey, "\n", `\n`) + require.NotContains(t, escaped, "\n") + inst, err := NewCloudLoggingDatasource(context.Background(), backend.DataSourceInstanceSettings{ + JSONData: []byte(jwtJSONData), + DecryptedSecureJSONData: map[string]string{"privateKey": escaped}, + }) + require.NoError(t, err) + require.NotNil(t, inst.(*CloudLoggingDatasource).client) + }) + + t.Run("key read from privateKeyPath", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "key.pem") + require.NoError(t, os.WriteFile(path, []byte(pemKey), 0o600)) + jsonData := strings.TrimSuffix(jwtJSONData, "}") + `, "privateKeyPath": "` + path + `"}` + inst, err := NewCloudLoggingDatasource(context.Background(), backend.DataSourceInstanceSettings{ + JSONData: []byte(jsonData), + }) + require.NoError(t, err) + require.NotNil(t, inst.(*CloudLoggingDatasource).client) + }) + + t.Run("unreadable privateKeyPath is reported", func(t *testing.T) { + jsonData := strings.TrimSuffix(jwtJSONData, "}") + `, "privateKeyPath": "/nonexistent/key.pem"}` + _, err := NewCloudLoggingDatasource(context.Background(), backend.DataSourceInstanceSettings{ + JSONData: []byte(jsonData), + }) + require.ErrorContains(t, err, "read private key") + }) + + t.Run("missing key", func(t *testing.T) { + _, err := NewCloudLoggingDatasource(context.Background(), backend.DataSourceInstanceSettings{ + JSONData: []byte(jwtJSONData), + }) + require.ErrorIs(t, err, errMissingCredentials) + }) + + t.Run("malformed key gets an explanatory error", func(t *testing.T) { + _, err := NewCloudLoggingDatasource(context.Background(), backend.DataSourceInstanceSettings{ + JSONData: []byte(jwtJSONData), + DecryptedSecureJSONData: map[string]string{"privateKey": "-----BEGIN PRIVATE KEY-----\nnot-a-key\n-----END PRIVATE KEY-----\n"}, + }) + require.ErrorContains(t, err, "create client") + require.ErrorContains(t, err, "including its line breaks") + }) +} + // responseSender implements backend.CallResourceResponseSender for testing type responseSender struct { resp *backend.CallResourceResponse From 454f28e8064af2dc2f478601b9dddc803556e4d1 Mon Sep 17 00:00:00 2001 From: Xiang Shen Date: Thu, 3 Sep 2026 22:38:24 +0000 Subject: [PATCH 5/5] address submission issues --- CHANGELOG.md | 2 ++ README.md | 10 +++++++++- go.mod | 4 ++-- go.sum | 8 ++++---- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b5afeb..e5a052a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ * Fix the query variable editor getting stuck on "Loading..." ([#212](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/212)). The editor no longer fetches buckets for an empty project or lists projects before they are needed; option lists load per scope, failures are shown inline instead of freezing the editor, and variable query errors (for example a disabled Cloud Resource Manager API) now surface in Grafana instead of silently producing no values. Selecting a scope is saved immediately * Fix an empty query text producing an unparseable filter (a leading `AND`); the time range alone is sent instead * Fix JWT authentication failing with "An error occurred within the plugin" when the data source is provisioned (YAML, Terraform, environment variables) and the private key contains literal `\n` escape sequences instead of line breaks ([#76](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/76), [#202](https://github.com/GoogleCloudPlatform/cloud-logging-data-source-plugin/issues/202)). The key is now read the same way as in the Google Cloud Monitoring data source, which also adds support for `privateKeyPath`. A key that still fails to parse produces an explanatory error instead of a generic one +* Update golang.org/x/crypto to v0.56.0 to address GO-2026-6303, GO-2026-6354 and GO-2026-6355 flagged by govulncheck in the Grafana plugin review. The remaining GO-2026-5932 advisory concerns the unmaintained `openpgp` package, which this plugin does not use and which has no fixed version +* README: link to public documentation for enabling the Cloud Resource Manager API and add the equivalent `gcloud` command ## 1.7.2 (2026-08-17) * Update dependencies to address security vulnerabilities flagged by the Grafana plugin review: js-yaml (CVE-2026-59869) and nanoid (CVE-2026-67213) in the frontend build toolchain diff --git a/README.md b/README.md index 9a5bee9..73557c8 100644 --- a/README.md +++ b/README.md @@ -15,9 +15,17 @@ You need to enable the resource manager API. Otherwise, your cloud projects will You can follow the steps to enable it: -1. Navigate to the [cloud resource manager API page](https://console.cloud.google.com/apis/library/cloudresourcemanager.googleapis.com) in GCP and select your project +1. In the Google Cloud console, open **APIs & Services > Library**, select your project and search for **Cloud Resource Manager API** 2. Press the `Enable` button +Alternatively, enable it from the command line: + +```sh +gcloud services enable cloudresourcemanager.googleapis.com --project= +``` + +See [Enabling and disabling services](https://cloud.google.com/service-usage/docs/enable-disable) for details. + ### Generate a JWT file & Assign IAM Permissions 1. If you don't have a GCP project, add a new GCP project [here](https://cloud.google.com/resource-manager/docs/creating-managing-projects#console) diff --git a/go.mod b/go.mod index bb35fb1..7d79fa6 100644 --- a/go.mod +++ b/go.mod @@ -99,12 +99,12 @@ require ( go.opentelemetry.io/otel/trace v1.44.0 // indirect go.opentelemetry.io/proto/otlp v1.11.0 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect - golang.org/x/crypto v0.54.0 // indirect + golang.org/x/crypto v0.56.0 // indirect golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect golang.org/x/net v0.57.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect - golang.org/x/text v0.40.0 // indirect + golang.org/x/text v0.41.0 // indirect golang.org/x/time v0.14.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260720211330-0afa2a65878a // indirect google.golang.org/grpc v1.83.1 // indirect diff --git a/go.sum b/go.sum index cdc1463..84444c0 100644 --- a/go.sum +++ b/go.sum @@ -266,8 +266,8 @@ go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= -golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/crypto v0.56.0 h1:GUh5Ii4J5jtcseSMiRqr1jXCNHoxjeV9Fmekc2oLy6Y= +golang.org/x/crypto v0.56.0/go.mod h1:OMW5y6CY9l38uPLmxU6l6pwcXp1obtLo3e6gT7gQR2I= golang.org/x/exp v0.0.0-20260112195511-716be5621a96 h1:Z/6YuSHTLOHfNFdb8zVZomZr7cqNgTJvA8+Qz75D8gU= golang.org/x/exp v0.0.0-20260112195511-716be5621a96/go.mod h1:nzimsREAkjBCIEFtHiYkrJyT+2uy9YZJB7H1k68CXZU= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= @@ -298,8 +298,8 @@ golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= -golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=