Feature/alerting business metrics

[riysaxen-amzn]

Add alerting_telemetry.ts helper module using OSD core TelemetryService (Neo browser telemetry) to emit custom events for business metrics.

P0 events instrumented:

• alerting.rule.created: after successful monitor/rule creation
• alerting.alert.acknowledged: after successful alert acknowledge
• alerting.slo.created: after successful SLO creation
• alerting.wizard.started: when create wizard opens
• alerting.wizard.completed: when wizard submission succeeds (with duration)

P1 event stubs included for future:

• alerting.rule.deleted, alerting.rule.edited
• alerting.slo.toggled, alerting.silence.created
• alerting.wizard.abandoned, alerting.detail.opened
• alerting.deeplink.from_mcp

Wired into:

• alarms_page.tsx: rule creation, acknowledge, wizard open
• slo_wizard_page.tsx: SLO creation

Description

[Describe what this change achieves]

Issues Resolved

[List any issues this PR will resolve]

Check List

• New functionality includes testing.
  • All tests pass, including unit test, integration test and doctest
• New functionality has been documented.
  • New functionality has javadoc added
  • New functionality has user manual doc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit b7ce6fe.

Path	Line	Severity	Description
public/components/alerting/alerting_telemetry.ts	47	low	Internal datasource IDs (dsId) are transmitted to the telemetry pipeline. While gated by isEnabled() and using the framework's own recorder, dsId values could reveal internal infrastructure topology to the telemetry backend. This is a data-minimization concern rather than a clear malicious act, but maintainers should confirm whether dsId needs to be sent or if a hashed/anonymized form suffices.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 0 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit f047753)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Style/Readability Two statements are combined on a single line using a semicolon inside the onCreateMonitor callback (setting wizardOpenTimeRef.current and calling alertingTelemetry.wizardStarted). This hurts readability and is inconsistent with the rest of the file. Split into separate lines. wizardOpenTimeRef.current = Date.now(); alertingTelemetry.wizardStarted({ entryPoint: 'button' }); } else if (type === 'metrics') { setCreateBackendType('prometheus'); setShowCreateMonitor(true); wizardOpenTimeRef.current = Date.now(); alertingTelemetry.wizardStarted({ entryPoint: 'button' }); Type Safety The public helpers advertise strict string-literal unions for dsType (e.g. 'opensearch' | 'prometheus' | 'mustang' | 'serverless'), but callers in alarms_page.tsx bypass this with as any (e.g. dsType: (alert?.datasourceType as any) || 'opensearch'). This defeats the type constraints and can silently emit unexpected dsType values into telemetry. Consider widening the type to string or validating/normalizing the value inside the helper. export function ruleCreated(data: { ruleType: 'ppl' | 'promql'; dsType: 'opensearch' | 'prometheus' | 'mustang' | 'serverless'; dsId: string; }): void { getRecorder()?.recordEvent({ name: 'alerting.rule.created', data, }); } /** * Emit when an alert is acknowledged (POST acknowledge returns 200). */ export function alertAcknowledged(data: { alertCount: number; dsType: 'opensearch' | 'prometheus'; }): void { getRecorder()?.recordEvent({ name: 'alerting.alert.acknowledged', data, }); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f047753
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Reset wizard timer after completion After emitting wizardCompleted, reset wizardOpenTimeRef.current to 0 to prevent stale timestamps from being used when the wizard is re-opened without triggering wizardStarted (e.g., via edit flow), which would produce misleadingly large durationMs values. public/components/alerting/alarms_page.tsx [946-949] alertingTelemetry.wizardCompleted({ ruleType: formState.datasourceType === 'prometheus' ? 'promql' : 'ppl', durationMs: wizardOpenTimeRef.current ? Date.now() - wizardOpenTimeRef.current : 0, }); +wizardOpenTimeRef.current = 0; Suggestion importance[1-10]: 4 __ Why: Resetting wizardOpenTimeRef.current after completion prevents stale timestamps, but the impact is minor since a new wizard open resets it via wizardStarted.	Low
	Split multi-statement lines for readability Avoid placing two statements on a single line joined by a semicolon; this harms readability and can trip linters/formatters. Split into separate lines for maintainability. public/components/alerting/alarms_page.tsx [1149-1153] setCreateBackendType('opensearch'); setShowCreateMonitor(true); - wizardOpenTimeRef.current = Date.now(); alertingTelemetry.wizardStarted({ entryPoint: 'button' }); + wizardOpenTimeRef.current = Date.now(); + alertingTelemetry.wizardStarted({ entryPoint: 'button' }); } else if (type === 'metrics') { setCreateBackendType('prometheus'); setShowCreateMonitor(true); - wizardOpenTimeRef.current = Date.now(); alertingTelemetry.wizardStarted({ entryPoint: 'button' }); + wizardOpenTimeRef.current = Date.now(); + alertingTelemetry.wizardStarted({ entryPoint: 'button' }); Suggestion importance[1-10]: 3 __ Why: A minor readability improvement to split combined statements onto separate lines; no functional impact.	Low
	Memoize telemetry recorder instance Each event emission calls getPluginRecorder anew, which may allocate a new recorder per call. Consider memoizing the recorder per core instance to reduce allocations and ensure batching consistency, while still handling the case where core is not yet initialized. public/components/alerting/alerting_telemetry.ts [24-32] +let cachedRecorder: PluginTelemetryRecorder | undefined; +let cachedCoreRef: unknown; function getRecorder(): PluginTelemetryRecorder | undefined { try { const telemetry = (coreRefs.core as any)?.telemetry; if (!telemetry || !telemetry.isEnabled()) return undefined; - return telemetry.getPluginRecorder(PLUGIN_ID); + if (cachedCoreRef !== coreRefs.core) { + cachedCoreRef = coreRefs.core; + cachedRecorder = telemetry.getPluginRecorder(PLUGIN_ID); + } + return cachedRecorder; } catch { return undefined; } } Suggestion importance[1-10]: 3 __ Why: Memoizing the recorder is a minor optimization; the overhead of getPluginRecorder is typically negligible and telemetry events are infrequent.	Low

---

Previous suggestions

Suggestions up to commit b7ce6fe

Category	Suggestion	Impact
Possible issue	Only emit wizard completion when started Emitting wizardCompleted regardless of whether the wizard was actually started (e.g. edit flow reuses the same submit path) will pollute the metric with durationMs: 0 entries. Only emit when wizardOpenTimeRef.current > 0, and reset it afterwards to prevent stale timestamps from leaking into future events. public/components/alerting/alarms_page.tsx [946-949] -alertingTelemetry.wizardCompleted({ - ruleType: formState.datasourceType === 'prometheus' ? 'promql' : 'ppl', - durationMs: wizardOpenTimeRef.current ? Date.now() - wizardOpenTimeRef.current : 0, -}); +if (wizardOpenTimeRef.current > 0) { + alertingTelemetry.wizardCompleted({ + ruleType: formState.datasourceType === 'prometheus' ? 'promql' : 'ppl', + durationMs: Date.now() - wizardOpenTimeRef.current, + }); + wizardOpenTimeRef.current = 0; +} Suggestion importance[1-10]: 7 __ Why: Correctly identifies that wizardCompleted can be emitted for edits or with stale timestamps, and the improved code properly guards and resets the ref to avoid misleading duration metrics.	Medium
General	Guard wizard duration telemetry accuracy The wizard start time is only recorded when opened via the "Create Monitor" button, but wizardCompleted is emitted on any successful submission (including edit flows). This will report durationMs: 0 for edits or when the ref was set from a previous session. Guard wizardCompleted emission on a non-zero wizardOpenTimeRef.current and reset it to 0 after emission to avoid misleading metrics. public/components/alerting/alarms_page.tsx [946-949] onCreateMonitor={(type) => { if (type === 'logs') { setCreateBackendType('opensearch'); setShowCreateMonitor(true); - wizardOpenTimeRef.current = Date.now(); alertingTelemetry.wizardStarted({ entryPoint: 'button' }); + wizardOpenTimeRef.current = Date.now(); + alertingTelemetry.wizardStarted({ entryPoint: 'button' }); } else if (type === 'metrics') { setCreateBackendType('prometheus'); setShowCreateMonitor(true); - wizardOpenTimeRef.current = Date.now(); alertingTelemetry.wizardStarted({ entryPoint: 'button' }); + wizardOpenTimeRef.current = Date.now(); + alertingTelemetry.wizardStarted({ entryPoint: 'button' }); } }} Suggestion importance[1-10]: 6 __ Why: Valid concern that wizardCompleted may emit with durationMs: 0 for edit flows or stale state, but the suggested improved_code only reformats the wizard-start block without actually adding the guard/reset it recommends.	Low
	Guard recordEvent calls against exceptions The P1 stub functions do not swallow exceptions themselves — they rely on getRecorder() returning undefined. If recordEvent throws (e.g. serialization error on fieldsChanged), the exception propagates to callers and can break user flows. Wrap recordEvent calls in a try/catch inside a shared helper to guarantee telemetry never affects UX. public/components/alerting/alerting_telemetry.ts [107-113] +function safeRecord(name: string, data: unknown): void { + try { + getRecorder()?.recordEvent({ name, data }); + } catch { + /* telemetry must never break UX */ + } +} + export function ruleDeleted(data: { ruleType: string; dsType: string }): void { - getRecorder()?.recordEvent({ name: 'alerting.rule.deleted', data }); + safeRecord('alerting.rule.deleted', data); } export function ruleEdited(data: { ruleType: string; dsType: string; fieldsChanged?: string[] }): void { - getRecorder()?.recordEvent({ name: 'alerting.rule.edited', data }); + safeRecord('alerting.rule.edited', data); } Suggestion importance[1-10]: 5 __ Why: Reasonable defensive improvement since getRecorder() only wraps its own logic in try/catch but not the recordEvent call itself; wrapping in a shared helper prevents telemetry errors from breaking UX flows.	Low

[github-actions]

Persistent review updated to latest commit f047753