Skip to content

Add httpRequestTimeoutMilliseconds option and default enableBaggage to false#214

Open
fpfp100 wants to merge 3 commits intomainfrom
users/pefan/exporter-timeout-fix
Open

Add httpRequestTimeoutMilliseconds option and default enableBaggage to false#214
fpfp100 wants to merge 3 commits intomainfrom
users/pefan/exporter-timeout-fix

Conversation

@fpfp100
Copy link
Contributor

@fpfp100 fpfp100 commented Mar 4, 2026

Summary

  • Agent365ExporterOptions: New httpRequestTimeoutMilliseconds option (default 30s) controls the per-HTTP-request timeout for backend calls, distinct from exporterTimeoutMilliseconds which is the BatchSpanProcessor export deadline.
  • Agent365ExporterOptions: exporterTimeoutMilliseconds default increased from 30s to 60s to allow sufficient time for retries across multiple identity groups.
  • ObservabilityHostingManager: enableBaggage now defaults to false instead of true. Callers must explicitly opt in with { enableBaggage: true }.

Test plan

  • All 1050 tests pass
  • Updated observability-hosting-manager.test.ts to reflect new default behavior
  • Build succeeds across all packages

🤖 Generated with Claude Code

- Agent365Exporter now uses the configured exporterTimeoutMilliseconds
  instead of a hardcoded 30s constant for HTTP request timeouts
- ObservabilityHostingManager.enableBaggage defaults to false (breaking);
  callers must opt in explicitly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fpfp100 fpfp100 requested review from a team as code owners March 4, 2026 22:30
Copilot AI review requested due to automatic review settings March 4, 2026 22:30
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes two observability configuration behaviors: ensuring Agent365Exporter honors its configured HTTP timeout, and changing ObservabilityHostingManager so baggage propagation middleware is opt-in by default.

Changes:

  • Respect Agent365ExporterOptions.exporterTimeoutMilliseconds when constructing the fetch abort timeout.
  • Change ObservabilityHostingManager.enableBaggage default behavior to false (opt-in).
  • Update tests and changelog to reflect the new defaults/behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/observability/extension/hosting/observability-hosting-manager.test.ts Updates expectations for the new default enableBaggage: false behavior and explicit opt-in.
packages/agents-a365-observability/src/tracing/exporter/Agent365Exporter.ts Removes hardcoded 30s timeout and uses configured exporterTimeoutMilliseconds.
packages/agents-a365-observability-hosting/src/middleware/ObservabilityHostingManager.ts Switches baggage middleware registration to opt-in (enableBaggage === true) and updates option docs.
CHANGELOG.md Documents the timeout fix and the behavior change for enableBaggage.
Comments suppressed due to low confidence (2)

packages/agents-a365-observability/src/tracing/exporter/Agent365Exporter.ts:250

  • The change to honor exporterTimeoutMilliseconds isn’t covered by an assertion in the existing Agent365Exporter test suite (no tests currently verify the fetch call’s abort signal/timeout). Adding a test that sets a non-default exporterTimeoutMilliseconds and asserts the fetch options use that value would prevent regressions back to a hardcoded timeout.
          body,
          signal: AbortSignal.timeout(this.options.exporterTimeoutMilliseconds)
        });

packages/agents-a365-observability/src/tracing/exporter/Agent365Exporter.ts:250

  • AbortSignal.timeout will throw if this.options.exporterTimeoutMilliseconds is undefined, NaN, or negative (e.g., if a caller passes a partial options object at runtime, or Object.assign overwrites the default with undefined). Consider validating/coercing the timeout before calling AbortSignal.timeout, and falling back to the default (30_000ms) when the value is not a finite positive number.
          body,
          signal: AbortSignal.timeout(this.options.exporterTimeoutMilliseconds)
        });

You can also share your feedback on Copilot code review. Take the survey.

nikhilNava
nikhilNava previously approved these changes Mar 5, 2026
threddy
threddy previously approved these changes Mar 5, 2026
juliomenendez
juliomenendez previously approved these changes Mar 5, 2026
- New httpRequestTimeoutMilliseconds option (default 30s) for per-HTTP-request
  timeout on fetch calls to the observability backend
- exporterTimeoutMilliseconds (BatchSpanProcessor deadline) increased to 60s
  to allow sufficient time for retries across multiple identity groups
- Clarified JSDoc to distinguish the two timeout semantics

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fpfp100 fpfp100 dismissed stale reviews from juliomenendez, threddy, and nikhilNava via 0ae2f6e March 5, 2026 20:52
@fpfp100 fpfp100 changed the title Fix exporter timeout config and default enableBaggage to false Add httpRequestTimeoutMilliseconds option and default enableBaggage to false Mar 5, 2026
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.

6 participants