diff --git a/.changeset/exemplars-agent-surface.md b/.changeset/exemplars-agent-surface.md new file mode 100644 index 0000000000..e529f57706 --- /dev/null +++ b/.changeset/exemplars-agent-surface.md @@ -0,0 +1,17 @@ +--- +'@hyperdx/api': minor +--- + +feat: accept exemplar settings on API- and agent-authored dashboard tiles + +`enableExemplars` and `exemplarTraceSourceId` are now accepted on line and +stacked-bar tiles through the external v2 API and the MCP dashboard tools, +survive the tile conversion in both directions, and are documented in the +OpenAPI spec. Previously they validated on write and were dropped before +persistence, so they could not be set through any surface. + +`exemplarTraceSourceId` is checked three ways, because a marker's "view trace" +link is only as good as the source behind it: an ObjectId on both surfaces, the +source must exist for the team, and it must actually be a Trace source. The +existence and kind checks mirror the heatmap gate. Without them a well-formed id +for a metric source saved cleanly and left every marker linking nowhere. diff --git a/packages/api/openapi.json b/packages/api/openapi.json index c2fbbb9aee..e9d21a8076 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -1648,6 +1648,16 @@ "minimum": 1, "description": "Maximum number of series rendered (top-N by value). Omit for no limit.", "example": 5 + }, + "enableExemplars": { + "type": "boolean", + "description": "Overlay exemplars: markers for individual trace-linked data points. Only renders when the tile is exemplar-eligible — a single non-ratio histogram metric series with no groupBy, aggregated with avg, min, max, quantile or last_value. Not count or sum: a counted point is not attributable to any one trace, so the overlay is accepted but stays inert.\n", + "default": false + }, + "exemplarTraceSourceId": { + "type": "string", + "description": "ID of the Trace source an exemplar marker links to. Must exist and be a Trace source; a request naming anything else is rejected. Defaults to the chart source's linked trace source when omitted.\n", + "example": "65f5e4a3b9e77c001a222222" } } }, @@ -1712,6 +1722,16 @@ "minimum": 1, "description": "Maximum number of series rendered (top-N by value). Omit for no limit.", "example": 5 + }, + "enableExemplars": { + "type": "boolean", + "description": "Overlay exemplars: markers for individual trace-linked data points. Only renders when the tile is exemplar-eligible — a single non-ratio histogram metric series with no groupBy, aggregated with avg, min, max, quantile or last_value. Not count or sum: a counted point is not attributable to any one trace, so the overlay is accepted but stays inert.\n", + "default": false + }, + "exemplarTraceSourceId": { + "type": "string", + "description": "ID of the Trace source an exemplar marker links to. Must exist and be a Trace source; a request naming anything else is rejected. Defaults to the chart source's linked trace source when omitted.\n", + "example": "65f5e4a3b9e77c001a222222" } } }, diff --git a/packages/api/package.json b/packages/api/package.json index 3a05a3d60e..bdc39bc1e6 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -92,7 +92,7 @@ "dev-task": "DOTENV_CONFIG_PATH=.env.development nodemon --exec 'ts-node' --transpile-only -r tsconfig-paths/register -r dotenv-expand/config -r '@hyperdx/node-opentelemetry/build/src/tracing' ./src/tasks/index.ts", "build": "rimraf ./build && tsc -p tsconfig.build.json && tsc-alias -p tsconfig.build.json && cp -r ./src/opamp/proto ./build/opamp/", "build:vercel": "rimraf ./build && tsc -p tsconfig.vercel.json && tsc-alias -p tsconfig.vercel.json && cp -r ./src/opamp/proto ./build/opamp/", - "lint": "npx eslint . --ext .ts --max-warnings 357", + "lint": "npx eslint . --ext .ts --max-warnings 358", "lint:fix": "npx eslint . --ext .ts --fix", "ci:lint": "yarn lint && yarn tsc --noEmit && yarn lint:openapi", "ci:unit": "jest --ci --coverage", diff --git a/packages/api/src/mcp/tools/dashboards/__tests__/exemplarSettings.test.ts b/packages/api/src/mcp/tools/dashboards/__tests__/exemplarSettings.test.ts new file mode 100644 index 0000000000..625b8027bc --- /dev/null +++ b/packages/api/src/mcp/tools/dashboards/__tests__/exemplarSettings.test.ts @@ -0,0 +1,85 @@ +import { Types } from 'mongoose'; + +import { mcpTilesParam } from '@/mcp/tools/dashboards/schemas'; + +/** + * `enableExemplars` and `exemplarTraceSourceId` are the two exemplar settings an + * MCP agent can put on a tile. The trace source id has to survive as something + * the source lookup can actually resolve — it is read back as a Mongo ObjectId — + * so accepting any string here would persist a tile whose markers silently never + * link to a trace. + */ +const tile = + (displayType: 'line' | 'stacked_bar') => + (config: Record) => [ + { + name: 'Latency', + config: { + displayType, + sourceId: new Types.ObjectId().toString(), + select: [{ aggFn: 'count', alias: 'Requests' }], + ...config, + }, + }, + ]; + +// The two tile types declare the exemplar fields in separate schema blocks, so +// an edit that only updates one would otherwise go unnoticed. +const lineTile = tile('line'); +const barTile = tile('stacked_bar'); + +describe('MCP tile exemplar settings', () => { + it('accepts a valid ObjectId as the exemplar trace source', () => { + const traceSourceId = new Types.ObjectId().toString(); + const parsed = mcpTilesParam.parse( + lineTile({ enableExemplars: true, exemplarTraceSourceId: traceSourceId }), + ); + expect(parsed[0].config).toMatchObject({ + enableExemplars: true, + exemplarTraceSourceId: traceSourceId, + }); + }); + + it.each([ + ['a source name rather than an id', 'Traces'], + ['a truncated id', '507f1f77bcf86cd7994390'], + ['an over-long id', '507f1f77bcf86cd799439011ff'], + ['an empty string', ''], + ['a non-hex id of the right length', 'zzzzzzzzzzzzzzzzzzzzzzzz'], + ])('rejects %s as the exemplar trace source', (_label, value) => { + expect(() => + mcpTilesParam.parse(lineTile({ exemplarTraceSourceId: value })), + ).toThrow(/Invalid ObjectId/); + }); + + // Worth recording because it is surprising: Mongo also accepts a 12-character + // string as 12 raw bytes, so `Types.ObjectId.isValid` — and therefore every + // objectIdSchema field in this codebase, not just this one — lets one through. + // Such an id simply resolves to no source, which is the same outcome as any + // other id that does not exist, so it is not worth diverging from the shared + // validator here. + it('lets a 12-character string through, as every other id field does', () => { + expect(() => + mcpTilesParam.parse(lineTile({ exemplarTraceSourceId: 'trace-source' })), + ).not.toThrow(); + }); + + it('leaves both settings optional', () => { + const parsed = mcpTilesParam.parse(lineTile({})); + expect(parsed[0].config).not.toHaveProperty('enableExemplars'); + expect(parsed[0].config).not.toHaveProperty('exemplarTraceSourceId'); + }); + + it.each([ + ['line', lineTile], + ['stacked_bar', barTile], + ])('validates the trace source on a %s tile', (_label, build) => { + expect(() => + mcpTilesParam.parse(build({ exemplarTraceSourceId: 'Traces' })), + ).toThrow(/Invalid ObjectId/); + const id = new Types.ObjectId().toString(); + expect( + mcpTilesParam.parse(build({ exemplarTraceSourceId: id }))[0].config, + ).toMatchObject({ exemplarTraceSourceId: id }); + }); +}); diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 75b85d89d9..9bdc0ab75f 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -543,6 +543,25 @@ const mcpLineTileSchema = mcpTileLayoutSchema.extend({ 'Scale the y-axis to the data range instead of starting at zero.', ), seriesLimit: seriesLimitSchema.describe(timeChartSeriesLimitDescription), + enableExemplars: z + .boolean() + .optional() + .describe( + 'Overlay exemplars: markers for individual trace-linked data points. ' + + 'Only renders on an exemplar-eligible tile — a single non-ratio ' + + 'histogram metric series with no groupBy, aggregated with avg, min, ' + + 'max, quantile or last_value. Not count or sum: a counted point is ' + + 'not attributable to any one trace, so the overlay stays inert with ' + + 'no error.', + ), + exemplarTraceSourceId: objectIdSchema + .optional() + .describe( + 'Trace source an exemplar marker links to. Must be an existing Trace ' + + 'source — use clickstack_list_sources and pick one whose kind is ' + + '"trace"; anything else is rejected on save. Defaults to the chart ' + + "source's linked trace source when omitted.", + ), }), }); @@ -561,6 +580,25 @@ const mcpBarTileSchema = mcpTileLayoutSchema.extend({ .optional() .describe(tileLevelNumberFormatDescription), seriesLimit: seriesLimitSchema.describe(timeChartSeriesLimitDescription), + enableExemplars: z + .boolean() + .optional() + .describe( + 'Overlay exemplars: markers for individual trace-linked data points. ' + + 'Only renders on an exemplar-eligible tile — a single non-ratio ' + + 'histogram metric series with no groupBy, aggregated with avg, min, ' + + 'max, quantile or last_value. Not count or sum: a counted point is ' + + 'not attributable to any one trace, so the overlay stays inert with ' + + 'no error.', + ), + exemplarTraceSourceId: objectIdSchema + .optional() + .describe( + 'Trace source an exemplar marker links to. Must be an existing Trace ' + + 'source — use clickstack_list_sources and pick one whose kind is ' + + '"trace"; anything else is rejected on save. Defaults to the chart ' + + "source's linked trace source when omitted.", + ), }), }); diff --git a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts index e860f3aba5..9d0e5ec1f0 100644 --- a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts @@ -2824,6 +2824,95 @@ describe('External API v2 Dashboards - new format', () => { ); }); + // `exemplarTraceSourceId` is where a marker's "view trace" link points, so a + // well-formed id for the wrong kind of source saves a tile whose markers all + // lead nowhere. Format validation alone cannot catch that. + it('rejects an exemplarTraceSourceId that is not a Trace source', async () => { + const response = await authRequest('post', BASE_URL) + .send({ + name: 'Dashboard with exemplars on a metric trace source', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: metricSource._id.toString(), + }, + }, + ], + tags: [], + }) + .expect(400); + + expect(response.body.message).toContain( + 'exemplarTraceSourceId must reference a Trace source', + ); + }); + + it('rejects an exemplarTraceSourceId for a source that does not exist', async () => { + const response = await authRequest('post', BASE_URL) + .send({ + name: 'Dashboard with exemplars on a missing source', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: new ObjectId().toString(), + }, + }, + ], + tags: [], + }) + .expect(400); + + expect(response.body.message).toContain('Could not find'); + }); + + it('accepts an exemplarTraceSourceId pointing at a Trace source', async () => { + const response = await authRequest('post', BASE_URL) + .send({ + name: 'Dashboard with exemplars', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: traceSource._id.toString(), + }, + }, + ], + tags: [], + }) + .expect(200); + + expect(response.body.data.tiles[0].config).toMatchObject({ + enableExemplars: true, + exemplarTraceSourceId: traceSource._id.toString(), + }); + }); + it('round-trips a heatmap tile with only required fields', async () => { // Covers the minimal payload path: countExpression, heatmapScaleType, // where, whereLanguage, and numberFormat are all omitted on the @@ -5037,6 +5126,245 @@ describe('External API v2 Dashboards - new format', () => { }) .expect(200); }); + + // Same reasoning as the heatmap case above: the trace source can be deleted + // or change kind long after the tile was accepted, and an optional marker + // link target must not make the whole dashboard unsaveable for edits made + // elsewhere on it. + it('does not re-validate the exemplar trace source for unchanged tiles', async () => { + const createResponse = await authRequest('post', BASE_URL) + .send({ + name: 'Exemplar PUT scoping test', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: traceSource._id.toString(), + }, + }, + { + name: 'Other line tile', + x: 6, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: traceSource._id.toString(), + select: [{ aggFn: 'count', where: '' }], + }, + }, + ], + tags: [], + }) + .expect(200); + + const dashboardId = createResponse.body.data.id; + const exemplarTile = createResponse.body.data.tiles.find( + (t: { name: string }) => t.name === 'Latency', + ); + const otherTile = createResponse.body.data.tiles.find( + (t: { name: string }) => t.name === 'Other line tile', + ); + + // The trace source stops being a Trace source after the fact. Written + // straight to the collection for the same reason the heatmap test does. + await Source.collection.updateOne( + { _id: traceSource._id }, + { $set: { kind: SourceKind.Log } }, + ); + + await authRequest('put', `${BASE_URL}/${dashboardId}`) + .send({ + name: 'Exemplar PUT scoping test - renamed', + tiles: [ + { ...exemplarTile }, + { + ...otherTile, + name: 'Other line tile, edited', + config: { + ...otherTile.config, + select: [{ aggFn: 'count', where: 'level:error' }], + }, + }, + ], + tags: [], + }) + .expect(200); + }); + + // Deletion, not just a kind change. These were split for a while: the kind + // gate was exempted for unchanged tiles and the existence check was not, so + // deleting the source still wedged every later save. Only the exemplar + // reference dangles here — both tiles sit on a source that still exists, so a + // rejection could only come from the exemplar check. + it('does not re-validate a deleted exemplar trace source for unchanged tiles', async () => { + const createResponse = await authRequest('post', BASE_URL) + .send({ + name: 'Exemplar deletion scoping test', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: traceSource._id.toString(), + }, + }, + { + name: 'Other line tile', + x: 6, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + }, + }, + ], + tags: [], + }) + .expect(200); + + const dashboardId = createResponse.body.data.id; + const exemplarTile = createResponse.body.data.tiles.find( + (t: { name: string }) => t.name === 'Latency', + ); + const otherTile = createResponse.body.data.tiles.find( + (t: { name: string }) => t.name === 'Other line tile', + ); + + await Source.collection.deleteOne({ _id: traceSource._id }); + + await authRequest('put', `${BASE_URL}/${dashboardId}`) + .send({ + name: 'Exemplar deletion scoping test - renamed', + tiles: [ + { ...exemplarTile }, + { + ...otherTile, + name: 'Other line tile, edited', + }, + ], + tags: [], + }) + .expect(200); + }); + }); + + describe('exemplar settings enabled on an existing tile', () => { + // The unchanged-tile exemption keys on the source id, but an id nobody was + // following is not the same as one about to draw markers: while exemplars + // were off the source could have been deleted or stopped being a Trace + // source with no effect at all. + it('validates the trace source when exemplars are switched on', async () => { + const createResponse = await authRequest('post', BASE_URL) + .send({ + name: 'Exemplar enable test', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: false, + exemplarTraceSourceId: traceSource._id.toString(), + }, + }, + ], + tags: [], + }) + .expect(200); + + const dashboardId = createResponse.body.data.id; + const tile = createResponse.body.data.tiles[0]; + + // The source stops being a Trace source while nothing was following it. + await Source.collection.updateOne( + { _id: traceSource._id }, + { $set: { kind: SourceKind.Log } }, + ); + + const response = await authRequest('put', `${BASE_URL}/${dashboardId}`) + .send({ + name: 'Exemplar enable test', + tiles: [ + { + ...tile, + config: { ...tile.config, enableExemplars: true }, + }, + ], + tags: [], + }) + .expect(400); + + expect(response.body.message).toContain( + 'exemplarTraceSourceId must reference a Trace source', + ); + }); + + it('still exempts a tile whose exemplars were already on', async () => { + const createResponse = await authRequest('post', BASE_URL) + .send({ + name: 'Exemplar already-on test', + tiles: [ + { + name: 'Latency', + x: 0, + y: 0, + w: 6, + h: 3, + config: { + displayType: 'line', + sourceId: metricSource._id.toString(), + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: traceSource._id.toString(), + }, + }, + ], + tags: [], + }) + .expect(200); + + const dashboardId = createResponse.body.data.id; + const tile = createResponse.body.data.tiles[0]; + + await Source.collection.updateOne( + { _id: traceSource._id }, + { $set: { kind: SourceKind.Log } }, + ); + + // enableExemplars stays true, so this is the exemption, not the transition. + await authRequest('put', `${BASE_URL}/${dashboardId}`) + .send({ + name: 'Exemplar already-on test - renamed', + tiles: [{ ...tile }], + tags: [], + }) + .expect(200); + }); }); describe('Number tile color (HDX-1360)', () => { diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index f3d92bed45..0f2efa03e0 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -634,6 +634,23 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * minimum: 1 * description: Maximum number of series rendered (top-N by value). Omit for no limit. * example: 5 + * enableExemplars: + * type: boolean + * description: > + * Overlay exemplars: markers for individual trace-linked data points. + * Only renders when the tile is exemplar-eligible — a single + * non-ratio histogram metric series with no groupBy, aggregated with + * avg, min, max, quantile or last_value. Not count or sum: a counted + * point is not attributable to any one trace, so the overlay is + * accepted but stays inert. + * default: false + * exemplarTraceSourceId: + * type: string + * description: > + * ID of the Trace source an exemplar marker links to. Must exist and + * be a Trace source; a request naming anything else is rejected. + * Defaults to the chart source's linked trace source when omitted. + * example: "65f5e4a3b9e77c001a222222" * * BarBuilderChartConfig: * type: object @@ -686,6 +703,23 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * minimum: 1 * description: Maximum number of series rendered (top-N by value). Omit for no limit. * example: 5 + * enableExemplars: + * type: boolean + * description: > + * Overlay exemplars: markers for individual trace-linked data points. + * Only renders when the tile is exemplar-eligible — a single + * non-ratio histogram metric series with no groupBy, aggregated with + * avg, min, max, quantile or last_value. Not count or sum: a counted + * point is not attributable to any one trace, so the overlay is + * accepted but stays inert. + * default: false + * exemplarTraceSourceId: + * type: string + * description: > + * ID of the Trace source an exemplar marker links to. Must exist and + * be a Trace source; a request naming anything else is rejected. + * Defaults to the chart source's linked trace source when omitted. + * example: "65f5e4a3b9e77c001a222222" * * TableBuilderChartConfig: * type: object diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts index 29618d8bb3..3abfae023f 100644 --- a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -400,3 +400,71 @@ describe('convertToExternalDashboard orphan-ref heal', () => { expect(ext.tiles.map(t => t.id)).toEqual(['normal-tile']); }); }); + +/** + * The exemplar settings are only useful if they survive the tile conversion in + * both directions — accepting them at the schema and dropping them in the + * converter was the original bug this feature fixed, and the two tile types have + * separate blocks in both converters, so one can silently regress without the + * other. + */ +describe('exemplar settings round-trip', () => { + const traceSourceId = new mongoose.Types.ObjectId().toString(); + const sourceId = new mongoose.Types.ObjectId().toString(); + + const externalTile = (displayType: 'line' | 'stacked_bar'): ConfigTile => ({ + id: 'tile-1', + x: 0, + y: 0, + w: 6, + h: 4, + name: 'Latency', + config: { + displayType, + sourceId, + select: [{ aggFn: 'avg', valueExpression: 'Duration' }], + enableExemplars: true, + exemplarTraceSourceId: traceSourceId, + } as ConfigTile['config'], + }); + + it.each(['line', 'stacked_bar'] as const)( + 'keeps both settings converting a %s tile inwards', + displayType => { + const { config } = convertToInternalTileConfig(externalTile(displayType)); + expect(config).toMatchObject({ + enableExemplars: true, + exemplarTraceSourceId: traceSourceId, + }); + }, + ); + + it.each(['line', 'stacked_bar'] as const)( + 'returns both settings for a %s tile on the way out', + displayType => { + const internal = convertToInternalTileConfig(externalTile(displayType)); + const dashboard = { + _id: new mongoose.Types.ObjectId(), + name: 'D', + tiles: [{ ...externalTile(displayType), config: internal.config }], + tags: [], + } as unknown as DashboardDocument; + + const [tile] = convertToExternalDashboard(dashboard).tiles; + expect(tile.config).toMatchObject({ + enableExemplars: true, + exemplarTraceSourceId: traceSourceId, + }); + }, + ); + + it('omits both when unset rather than emitting nulls', () => { + const bare = externalTile('line'); + delete (bare.config as Record).enableExemplars; + delete (bare.config as Record).exemplarTraceSourceId; + + const { config } = convertToInternalTileConfig(bare); + expect(config).not.toHaveProperty('enableExemplars'); + expect(config).not.toHaveProperty('exemplarTraceSourceId'); + }); +}); diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index 42522b0abe..2b36964987 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -300,6 +300,8 @@ const convertToExternalTileChartConfig = ( compareToPreviousPeriod: config.compareToPreviousPeriod, numberFormat: config.numberFormat, seriesLimit: config.seriesLimit ?? undefined, + enableExemplars: config.enableExemplars, + exemplarTraceSourceId: config.exemplarTraceSourceId, }; case DisplayType.StackedBar: return { @@ -317,6 +319,8 @@ const convertToExternalTileChartConfig = ( : [DEFAULT_SELECT_ITEM], numberFormat: config.numberFormat, seriesLimit: config.seriesLimit ?? undefined, + enableExemplars: config.enableExemplars, + exemplarTraceSourceId: config.exemplarTraceSourceId, }; case DisplayType.Number: return { @@ -694,6 +698,8 @@ export function convertToInternalTileConfig( 'alignDateRangeToGranularity', 'compareToPreviousPeriod', 'fitYAxisToData', + 'enableExemplars', + 'exemplarTraceSourceId', ]), displayType: externalConfig.displayType === 'stacked_bar' @@ -937,6 +943,9 @@ function getMissingSources( if ('sourceId' in tile.config && tile.config.sourceId) { sourceIds.add(tile.config.sourceId); } + // exemplarTraceSourceId is deliberately NOT collected here: it is checked + // separately so the same unchanged-tile exemption can apply to it. See + // getExemplarTraceSourceIssues. } // Include source IDs referenced by OnClick link-outs (mode=id, type=search) @@ -991,6 +1000,46 @@ function getHeatmapTilesWithIncompatibleSources( }); } +/** + * Both problems an exemplar trace source can have: it does not exist, or it + * exists and is not a Trace source. + * + * Checked here rather than folding existence into getMissingSources so one + * unchanged-tile exemption covers both. Splitting them meant a deleted source + * blocked an update while a source whose kind had changed did not, which is not a + * distinction anyone could predict. + * + * Unlike a tile's `sourceId`, this reference is optional decoration: the chart + * renders the same without it, only a marker's "view trace" link goes dead. That + * is not worth refusing an edit made elsewhere on the dashboard over. + */ +function getExemplarTraceSourceIssues( + sources: SourceForValidation[], + tiles: ExternalDashboardTileWithId[], +): { missing: string[]; notTrace: string[] } { + const traceSourceIds = new Set(); + for (const tile of tiles) { + if ( + isConfigTile(tile) && + 'exemplarTraceSourceId' in tile.config && + tile.config.exemplarTraceSourceId + ) { + traceSourceIds.add(tile.config.exemplarTraceSourceId); + } + } + if (traceSourceIds.size === 0) return { missing: [], notTrace: [] }; + + const sourceById = new Map(sources.map(s => [s._id.toString(), s])); + const missing: string[] = []; + const notTrace: string[] = []; + for (const id of traceSourceIds) { + const source = sourceById.get(id); + if (source === undefined) missing.push(id); + else if (!isTraceSource(source)) notTrace.push(id); + } + return { missing, notTrace }; +} + /** * For a PUT (update) request, return only the heatmap tiles that need * to be re-validated against the source-kind gate. A heatmap tile that @@ -1036,6 +1085,51 @@ function filterChangedHeatmapTiles( }); } +/** + * For a PUT (update), return only the tiles whose exemplar trace source needs + * re-checking: new tiles, and existing ones where the id actually changed. + * + * Same reasoning as filterChangedHeatmapTiles. Without it, a tile whose trace + * source was later deleted or changed kind would fail the gate on every + * subsequent save, so an unrelated edit elsewhere on the dashboard could not be + * persisted at all. + */ +function filterChangedExemplarTiles( + requestTiles: ExternalDashboardTileWithId[], + existingTiles: DashboardDocument['tiles'], +): ExternalDashboardTileWithId[] { + const existingTilesById = new Map( + existingTiles.map(t => [t.id, t]), + ); + return requestTiles.filter(tile => { + if ( + !isConfigTile(tile) || + !('exemplarTraceSourceId' in tile.config) || + !tile.config.exemplarTraceSourceId + ) { + return false; + } + const existing = tile.id ? existingTilesById.get(tile.id) : undefined; + // A new tile, or one that had no exemplar trace source before: validate. + if (existing === undefined) return true; + const existingConfig = existing.config; + if (isRawSqlSavedChartConfig(existingConfig)) return true; + if ( + existingConfig.exemplarTraceSourceId !== tile.config.exemplarTraceSourceId + ) { + return true; + } + // The id is unchanged, but a reference nobody was following is not the same + // as one that is about to draw markers. While exemplars were off the source + // could have been deleted or stopped being a Trace source with no effect, so + // switching them on has to be checked as if the reference were new. + return ( + tile.config.enableExemplars === true && + existingConfig.enableExemplars !== true + ); + }); +} + /** * Returns source IDs referenced by onClick search link-outs (mode=id, * type=search) whose source kind is not log or trace. The /search destination @@ -1303,6 +1397,23 @@ export async function validateDashboardTiles( return `Heatmap tiles require a Trace source. The following source IDs are not Trace sources: ${heatmapNonTraceSources.join(', ')}`; } + // Scoped to changed tiles on update, like the heatmap gate above: an unchanged + // tile whose trace source has since been deleted must not block edits made + // elsewhere on the dashboard. + const exemplarTilesToCheck = existingTiles + ? filterChangedExemplarTiles(tiles, existingTiles) + : tiles; + const exemplarTraceSourceIssues = getExemplarTraceSourceIssues( + sources, + exemplarTilesToCheck, + ); + if (exemplarTraceSourceIssues.missing.length > 0) { + return `Could not find the following exemplarTraceSourceId source IDs: ${exemplarTraceSourceIssues.missing.join(', ')}`; + } + if (exemplarTraceSourceIssues.notTrace.length > 0) { + return `exemplarTraceSourceId must reference a Trace source. The following source IDs are not Trace sources: ${exemplarTraceSourceIssues.notTrace.join(', ')}`; + } + if (missingOnClickDashboards.length > 0) { return `Could not find the following onClick dashboard IDs: ${missingOnClickDashboards.join(', ')}`; } diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 756090b69c..739514a048 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -261,6 +261,11 @@ const externalDashboardTimeChartConfigSchema = z.object({ alignDateRangeToGranularity: z.boolean().optional(), fillNulls: z.boolean().optional(), numberFormat: NumberFormatSchema.optional(), + // Exemplar overlay (trace-linked markers). Rendering additionally requires an + // exemplar-eligible shape — single non-ratio histogram series, no group by — + // so setting this on an ineligible tile is inert rather than an error. + enableExemplars: z.boolean().optional(), + exemplarTraceSourceId: objectIdSchema.optional(), }); const externalDashboardLineChartConfigSchema =