Skip to content

feat(perf): report SDK benchmarks as named OTLP log events - #246

Merged
Jackson Weber (JacksonWeber) merged 3 commits into
microsoft:mainfrom
JacksonWeber:jacksonweber-microsoft-javascript-performance-telemetry
Sep 28, 2026
Merged

Jackson Weber (JacksonWeber) merged 3 commits into
microsoft:mainfrom
JacksonWeber:jacksonweber-microsoft-javascript-performance-telemetry

Conversation

@JacksonWeber

Copy link
Copy Markdown
Contributor

Measure genuine SDK throughput and signed memory deltas, preserve raw artifacts, and generate named OTLP log results with explicitly configured export.

Measure genuine SDK throughput and signed memory deltas, preserve raw artifacts, and generate named OTLP log results with explicitly configured export.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings affect benchmark reliability, artifact preservation, and export correctness.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
What changed in this PR

Adds SDK performance benchmarking with signed memory measurements, raw artifact validation, and explicit OTLP log-event export.

Changes:

  • Adds span, metric, and log throughput benchmarks.
  • Validates and preserves raw benchmark artifacts.
  • Generates and exports named OTLP log events.
  • Documents benchmark workflows.
File Summary
test/​integration/​performance-events.test.mjs Tests benchmarking, validation, and export behavior.
perf/​report-results.mjs Validates artifacts and creates OTLP events. Moderate finding: scenario identity fields can be relabeled.
perf/​memory-worker.mjs Captures isolated memory measurements.
perf/​export-events.mjs Writes and exports event payloads. Critical finding: aliased output paths can overwrite raw artifacts. Moderate finding: valid empty 2xx responses are rejected.
perf/​benchmark.mjs Runs throughput and memory benchmarks.
perf/​benchmark-sdk.mjs Configures isolated SDK workloads. Critical finding: the metric reader uses an abstract API incorrectly. Moderate finding: detector suppression may target the wrong dependency tree.
CONTRIBUTING.md Documents benchmark and export workflows.
CHANGELOG.md Records the new performance tooling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread perf/benchmark-sdk.mjs
Comment thread perf/export-events.mjs
Reject input/output file identity aliases and atomically replace generated payloads without writing through output links.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Scenario validation permits fabricated test and category identities in exported events.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Validate all canonical scenario identity fields

perf/​report-results.mjs:102

The validation only compares scenario names, so a raw artifact can change a scenario's test and category (and make the same change in its memory entry) and still emit records under fabricated identities. Compare all three canonical identity fields from scenarios, not just name, so the named events cannot be relabeled while passing validation.

Comment thread perf/report-results.mjs
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@JacksonWeber
Jackson Weber (JacksonWeber) merged commit 7d427b2 into microsoft:main Sep 28, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants