Skip to content

Commit fde3489

Browse files
committed
fix(dynatrace): close the gaps a validation pass turned up
Three real defects and one usability gap, all found by auditing the tools against the Dynatrace API reference a second time. `ingest_logs` double-encoded its payload. `logs` is a `json` param, and a `json` param arrives as a *string* whenever it comes from a long-input field or an LLM tool call — only a block-to-block reference hands over a parsed value. `JSON.stringify` on that string produced `"[{...}]"`, so Dynatrace received a quoted string where it expected an array. The block hid this in the UI path by pre-parsing, but the parse lived in `tools.config.params` and *threw* on malformed input, and it never covered the direct tool-call path at all. Both tools now normalize through the shared `parseJsonParam`, so the tool is correct regardless of who calls it, and the block just forwards the raw value. `ingest_event.properties` had the identical bug. Path identifiers were not trimmed. A problem or entity ID pasted with a trailing newline became `%0A` in the URL and 404'd with nothing to suggest whitespace was the cause. Errors dropped the part that matters. Dynatrace's ErrorEnvelope carries `constraintViolations[]`, which names the offending selector or parameter; the generic `nested-error-object` extractor returns only `error.message` ("Constraints violated."), and which extractor won was left to fallback order. Adds a `dynatrace-errors` extractor that folds the violations into the message and pins it on all 22 tools. It sits after `nested-error-object` in the chain, which already matches this shape, so no other service's error handling changes. Adds 21 tests covering URL construction for SaaS/Managed/ActiveGate, cursor pagination dropping sibling filters, identifier trimming, metric-key colon preservation, both JSON-param paths, the `eventTimeout` -> `timeout` mapping, EntityStub flattening, the audit log's dotted `dt.settings.*` keys, and the 204/200 split on log ingestion.
1 parent 9185a43 commit fde3489

29 files changed

Lines changed: 402 additions & 37 deletions

apps/docs/content/docs/en/integrations/dynatrace.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ Discover the metrics available in a Dynatrace environment, filtered by metric se
173173
| `apiToken` | string | Yes | Dynatrace access token \(dt0c01...\) with the metrics.read scope |
174174
| `metricSelector` | string | No | Metric selector, supporting wildcards \(e.g. builtin:host.cpu.*\) |
175175
| `text` | string | No | Free-text search across metric display names and descriptions |
176-
| `fields` | string | No | Comma-separated descriptor properties to include. metricId is always returned \(e.g. +displayName,+unit,+aggregationTypes\) |
176+
| `fields` | string | No | Comma-separated descriptor properties. Prefix with + to add a non-default property and - to drop a default one; metricId is always returned \(e.g. +aggregationTypes,-description\) |
177177
| `writtenSince` | string | No | Only metrics written since this point, as UTC milliseconds, ISO 8601, or a relative expression such as now-7d |
178178
| `writtenSinceMode` | string | No | INCLUDE \(default\) keeps metrics written since Written Since; EXCLUDE keeps the ones not written since then |
179179
| `metadataSelector` | string | No | Metadata selector, e.g. unit\("Percent"\),tags\("dashboard"\) |

apps/sim/blocks/blocks/dynatrace.ts

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ Return ONLY the selector string - no explanations, no surrounding quotes.`,
260260
id: 'metricFields',
261261
title: 'Additional Fields',
262262
type: 'short-input',
263-
placeholder: '+displayName,+unit,+aggregationTypes',
263+
placeholder: '+aggregationTypes,-description',
264264
mode: 'advanced',
265265
condition: { field: 'operation', value: 'dynatrace_list_metrics' },
266266
},
@@ -790,16 +790,6 @@ Return ONLY the selector string - no explanations, no surrounding quotes.`,
790790
const toNumber = (value: unknown) =>
791791
value === undefined || value === null || value === '' ? undefined : Number(value)
792792

793-
const parseJson = (value: unknown) => {
794-
if (value === undefined || value === null || value === '') return undefined
795-
if (typeof value !== 'string') return value
796-
try {
797-
return JSON.parse(value)
798-
} catch {
799-
throw new Error('Value must be valid JSON')
800-
}
801-
}
802-
803793
const pagination = {
804794
pageSize: toNumber(params.pageSize),
805795
nextPageKey: params.nextPageKey || undefined,
@@ -913,7 +903,7 @@ Return ONLY the selector string - no explanations, no surrounding quotes.`,
913903
startTime: toNumber(params.startTime),
914904
endTime: toNumber(params.endTime),
915905
eventTimeout: toNumber(params.eventTimeout),
916-
properties: parseJson(params.eventProperties),
906+
properties: params.eventProperties || undefined,
917907
}
918908

919909
case 'dynatrace_search_logs':
@@ -928,7 +918,7 @@ Return ONLY the selector string - no explanations, no surrounding quotes.`,
928918
}
929919

930920
case 'dynatrace_ingest_logs':
931-
return { ...baseParams, logs: parseJson(params.logs) }
921+
return { ...baseParams, logs: params.logs }
932922

933923
case 'dynatrace_list_slos':
934924
return {

apps/sim/tools/dynatrace/add_problem_comment.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import type {
22
DynatraceAddProblemCommentParams,
33
DynatraceAddProblemCommentResponse,
44
} from '@/tools/dynatrace/types'
5-
import { buildDynatraceUrl, dynatraceHeaders } from '@/tools/dynatrace/utils'
5+
import { buildDynatraceUrl, dynatraceHeaders, encodeDynatraceId } from '@/tools/dynatrace/utils'
6+
import { ErrorExtractorId } from '@/tools/error-extractors'
67
import type { ToolConfig } from '@/tools/types'
78

89
export const addProblemCommentTool: ToolConfig<
@@ -13,6 +14,7 @@ export const addProblemCommentTool: ToolConfig<
1314
name: 'Dynatrace Add Problem Comment',
1415
description: 'Add a comment to a Dynatrace problem.',
1516
version: '1.0.0',
17+
errorExtractor: ErrorExtractorId.DYNATRACE_ERRORS,
1618

1719
params: {
1820
environmentUrl: {
@@ -52,7 +54,7 @@ export const addProblemCommentTool: ToolConfig<
5254
url: (params) =>
5355
buildDynatraceUrl(
5456
params.environmentUrl,
55-
`/problems/${encodeURIComponent(params.problemId)}/comments`
57+
`/problems/${encodeDynatraceId(params.problemId)}/comments`
5658
),
5759
method: 'POST',
5860
headers: (params) => dynatraceHeaders(params.apiToken),

apps/sim/tools/dynatrace/close_problem.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ import type {
66
import {
77
buildDynatraceUrl,
88
dynatraceHeaders,
9+
encodeDynatraceId,
910
mapComment,
1011
readJsonBody,
1112
} from '@/tools/dynatrace/utils'
13+
import { ErrorExtractorId } from '@/tools/error-extractors'
1214
import type { ToolConfig } from '@/tools/types'
1315

1416
export const closeProblemTool: ToolConfig<
@@ -19,6 +21,7 @@ export const closeProblemTool: ToolConfig<
1921
name: 'Dynatrace Close Problem',
2022
description: 'Close a Dynatrace problem and record the closing comment.',
2123
version: '1.0.0',
24+
errorExtractor: ErrorExtractorId.DYNATRACE_ERRORS,
2225

2326
params: {
2427
environmentUrl: {
@@ -52,7 +55,7 @@ export const closeProblemTool: ToolConfig<
5255
url: (params) =>
5356
buildDynatraceUrl(
5457
params.environmentUrl,
55-
`/problems/${encodeURIComponent(params.problemId)}/close`
58+
`/problems/${encodeDynatraceId(params.problemId)}/close`
5659
),
5760
method: 'POST',
5861
headers: (params) => dynatraceHeaders(params.apiToken),
Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { getAuditLogsTool } from '@/tools/dynatrace/get_audit_logs'
6+
import { getEntityTool } from '@/tools/dynatrace/get_entity'
7+
import { getMetricTool } from '@/tools/dynatrace/get_metric'
8+
import { getProblemTool } from '@/tools/dynatrace/get_problem'
9+
import { ingestEventTool } from '@/tools/dynatrace/ingest_event'
10+
import { ingestLogsTool } from '@/tools/dynatrace/ingest_logs'
11+
import { listProblemsTool } from '@/tools/dynatrace/list_problems'
12+
import { buildDynatraceUrl, dynatraceHeaders } from '@/tools/dynatrace/utils'
13+
import { ErrorExtractorId, extractErrorMessageWithId } from '@/tools/error-extractors'
14+
15+
const ENV = 'https://abc12345.live.dynatrace.com'
16+
const TOKEN = 'dt0c01.TOKEN'
17+
18+
function url(tool: { request: { url: string | ((p: never) => string) } }, params: object) {
19+
const build = tool.request.url
20+
return typeof build === 'function' ? build(params as never) : build
21+
}
22+
23+
function body(tool: { request: { body?: (p: never) => unknown } }, params: object) {
24+
return tool.request.body?.(params as never)
25+
}
26+
27+
describe('buildDynatraceUrl', () => {
28+
it('appends the v2 API path to a SaaS environment URL', () => {
29+
expect(buildDynatraceUrl(ENV, '/problems')).toBe(`${ENV}/api/v2/problems`)
30+
})
31+
32+
it('tolerates a trailing slash and a trailing /api/v2 segment', () => {
33+
expect(buildDynatraceUrl(`${ENV}/`, '/problems')).toBe(`${ENV}/api/v2/problems`)
34+
expect(buildDynatraceUrl(`${ENV}/api/v2`, '/problems')).toBe(`${ENV}/api/v2/problems`)
35+
expect(buildDynatraceUrl(` ${ENV}/api/v2/ `, '/problems')).toBe(`${ENV}/api/v2/problems`)
36+
})
37+
38+
it('keeps the environment path of a Managed / ActiveGate URL', () => {
39+
expect(buildDynatraceUrl('https://ag.example.com:9999/e/abc12345', '/problems')).toBe(
40+
'https://ag.example.com:9999/e/abc12345/api/v2/problems'
41+
)
42+
})
43+
44+
it('omits unset and empty query params but keeps false', () => {
45+
expect(
46+
buildDynatraceUrl(ENV, '/slo', {
47+
sloSelector: undefined,
48+
sort: '',
49+
from: null,
50+
evaluate: false,
51+
pageSize: 10,
52+
})
53+
).toBe(`${ENV}/api/v2/slo?evaluate=false&pageSize=10`)
54+
})
55+
})
56+
57+
describe('auth header', () => {
58+
it('uses the Api-Token scheme and trims the token', () => {
59+
expect(dynatraceHeaders(` ${TOKEN} `).Authorization).toBe(`Api-Token ${TOKEN}`)
60+
})
61+
})
62+
63+
describe('path identifiers', () => {
64+
const base = { environmentUrl: ENV, apiToken: TOKEN }
65+
66+
it('trims whitespace pasted around an identifier', () => {
67+
expect(url(getProblemTool, { ...base, problemId: ' P-123_456V2 ' })).toBe(
68+
`${ENV}/api/v2/problems/P-123_456V2`
69+
)
70+
expect(url(getEntityTool, { ...base, entityId: ' HOST-06F288EE2A930951\n' })).toBe(
71+
`${ENV}/api/v2/entities/HOST-06F288EE2A930951`
72+
)
73+
})
74+
75+
it('leaves the colon separators of a metric key unencoded', () => {
76+
expect(url(getMetricTool, { ...base, metricKey: ' builtin:host.cpu.usage:avg ' })).toBe(
77+
`${ENV}/api/v2/metrics/builtin:host.cpu.usage:avg`
78+
)
79+
})
80+
81+
it('drops every other filter once a page cursor is supplied', () => {
82+
expect(
83+
url(listProblemsTool, { ...base, nextPageKey: 'CURSOR', from: 'now-7d', pageSize: 500 })
84+
).toBe(`${ENV}/api/v2/problems?nextPageKey=CURSOR`)
85+
})
86+
})
87+
88+
describe('json request params', () => {
89+
const base = { environmentUrl: ENV, apiToken: TOKEN }
90+
91+
it('sends a JSON-string log payload as JSON, not as a quoted string', () => {
92+
const sent = body(ingestLogsTool, {
93+
...base,
94+
logs: '[{"content":"Deploy finished","severity":"info"}]',
95+
})
96+
expect(sent).toBe('[{"content":"Deploy finished","severity":"info"}]')
97+
expect(JSON.parse(sent as string)).toEqual([{ content: 'Deploy finished', severity: 'info' }])
98+
})
99+
100+
it('sends an already-parsed log payload unchanged', () => {
101+
const sent = body(ingestLogsTool, { ...base, logs: [{ content: 'hi' }] })
102+
expect(JSON.parse(sent as string)).toEqual([{ content: 'hi' }])
103+
})
104+
105+
it('parses a JSON-string properties object on event ingest', () => {
106+
const sent = body(ingestEventTool, {
107+
...base,
108+
eventType: 'CUSTOM_DEPLOYMENT',
109+
title: 'Deploy 4.12.2',
110+
properties: '{"version":"4.12.2"}',
111+
}) as Record<string, unknown>
112+
expect(sent.properties).toEqual({ version: '4.12.2' })
113+
})
114+
115+
it('maps the event timeout onto Dynatrace’s timeout field', () => {
116+
const sent = body(ingestEventTool, {
117+
...base,
118+
eventType: 'CUSTOM_INFO',
119+
title: 'x',
120+
eventTimeout: 30,
121+
}) as Record<string, unknown>
122+
expect(sent.timeout).toBe(30)
123+
expect(sent.eventTimeout).toBeUndefined()
124+
})
125+
126+
it('omits optional event fields that were not provided', () => {
127+
const sent = body(ingestEventTool, {
128+
...base,
129+
eventType: 'CUSTOM_INFO',
130+
title: 'x',
131+
}) as Record<string, unknown>
132+
expect(Object.keys(sent).sort()).toEqual(['eventType', 'title'])
133+
})
134+
})
135+
136+
describe('error extraction', () => {
137+
const extract = (data: unknown) =>
138+
extractErrorMessageWithId({ status: 400, data } as never, ErrorExtractorId.DYNATRACE_ERRORS)
139+
140+
it('names the offending parameter from constraintViolations', () => {
141+
expect(
142+
extract({
143+
error: {
144+
code: 400,
145+
message: 'Constraints violated.',
146+
constraintViolations: [
147+
{ path: 'metricSelector', message: "Unknown metric key 'builtin:bogus'." },
148+
],
149+
},
150+
})
151+
).toBe("Constraints violated. (metricSelector: Unknown metric key 'builtin:bogus'.)")
152+
})
153+
154+
it('joins several violations', () => {
155+
expect(
156+
extract({
157+
error: {
158+
message: 'Constraints violated.',
159+
constraintViolations: [
160+
{ path: 'from', message: 'Invalid timeframe.' },
161+
{ path: 'pageSize', message: 'Must be at most 500.' },
162+
],
163+
},
164+
})
165+
).toBe('Constraints violated. (from: Invalid timeframe.; pageSize: Must be at most 500.)')
166+
})
167+
168+
it('falls back to the bare message when there are no violations', () => {
169+
expect(extract({ error: { code: 404, message: 'Problem not found.' } })).toBe(
170+
'Problem not found.'
171+
)
172+
})
173+
174+
it('every Dynatrace tool pins the extractor so selection is deterministic', () => {
175+
for (const tool of [
176+
listProblemsTool,
177+
getProblemTool,
178+
getEntityTool,
179+
getMetricTool,
180+
getAuditLogsTool,
181+
ingestEventTool,
182+
ingestLogsTool,
183+
]) {
184+
expect(tool.errorExtractor).toBe(ErrorExtractorId.DYNATRACE_ERRORS)
185+
}
186+
})
187+
})
188+
189+
describe('response mapping', () => {
190+
it('flattens nested EntityStub ids and normalizes absent optional blocks', async () => {
191+
const response = new Response(
192+
JSON.stringify({
193+
totalCount: 1,
194+
pageSize: 50,
195+
nextPageKey: null,
196+
problems: [
197+
{
198+
problemId: 'P-1',
199+
title: 'CPU saturation',
200+
status: 'OPEN',
201+
endTime: -1,
202+
rootCauseEntity: { entityId: { id: 'HOST-1', type: 'HOST' }, name: 'web-01' },
203+
affectedEntities: [{ entityId: { id: 'SERVICE-1', type: 'SERVICE' }, name: 'api' }],
204+
},
205+
],
206+
}),
207+
{ status: 200 }
208+
)
209+
210+
const result = await listProblemsTool.transformResponse!(response)
211+
const problem = result.output.problems[0]
212+
213+
expect(problem.rootCauseEntity).toEqual({ id: 'HOST-1', type: 'HOST', name: 'web-01' })
214+
expect(problem.affectedEntities).toEqual([{ id: 'SERVICE-1', type: 'SERVICE', name: 'api' }])
215+
expect(problem.endTime).toBe(-1)
216+
expect(problem.impactedEntities).toEqual([])
217+
expect(problem.evidenceDetails).toBeNull()
218+
expect(result.output.nextPageKey).toBeNull()
219+
expect(result.output.warnings).toEqual([])
220+
})
221+
222+
it('lifts the dotted dt.settings keys of an audit entry into camelCase', async () => {
223+
const response = new Response(
224+
JSON.stringify({
225+
auditLogs: [
226+
{
227+
logId: 'L-1',
228+
user: 'someone@example.com',
229+
success: true,
230+
'dt.settings.schema_id': 'builtin:alerting.profile',
231+
'dt.settings.object_id': 'OBJ-1',
232+
},
233+
],
234+
}),
235+
{ status: 200 }
236+
)
237+
238+
const result = await getAuditLogsTool.transformResponse!(response)
239+
expect(result.output.auditLogs[0].settingsSchemaId).toBe('builtin:alerting.profile')
240+
expect(result.output.auditLogs[0].settingsObjectId).toBe('OBJ-1')
241+
expect(result.output.auditLogs[0].message).toBeNull()
242+
})
243+
244+
it('reads a 204 log ingestion as fully accepted despite the empty body', async () => {
245+
const result = await ingestLogsTool.transformResponse!(new Response(null, { status: 204 }))
246+
expect(result.output).toEqual({ accepted: true, statusCode: 204, details: null })
247+
})
248+
249+
it('surfaces a 200 partial-success log ingestion body', async () => {
250+
const response = new Response(JSON.stringify({ error: { message: 'some invalid' } }), {
251+
status: 200,
252+
})
253+
const result = await ingestLogsTool.transformResponse!(response)
254+
expect(result.output.accepted).toBe(false)
255+
expect(result.output.details).toEqual({ error: { message: 'some invalid' } })
256+
})
257+
})

apps/sim/tools/dynatrace/get_audit_logs.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
mapAuditLog,
1010
readJsonBody,
1111
} from '@/tools/dynatrace/utils'
12+
import { ErrorExtractorId } from '@/tools/error-extractors'
1213
import type { ToolConfig } from '@/tools/types'
1314

1415
export const getAuditLogsTool: ToolConfig<
@@ -20,6 +21,7 @@ export const getAuditLogsTool: ToolConfig<
2021
description:
2122
'Read the Dynatrace audit log — who changed which configuration, when, and whether it succeeded.',
2223
version: '1.0.0',
24+
errorExtractor: ErrorExtractorId.DYNATRACE_ERRORS,
2325

2426
params: {
2527
environmentUrl: {

0 commit comments

Comments
 (0)