Remove newly added md files under observability by fpfp100 · Pull Request #191 · microsoft/Agent365-nodejs

fpfp100 · 2026-02-09T19:47:24Z

Remove newly added md files under observability

Copilot

Pull request overview

Removes recently added documentation markdown files from the agents-a365-observability package.

Changes:

Deleted PER_REQUEST_EXPORT.md
Deleted LOGGER_SETUP.md

Reviewed changes

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

File	Description
packages/agents-a365-observability/PER_REQUEST_EXPORT.md	Removes per-request export documentation from the package.
packages/agents-a365-observability/LOGGER_SETUP.md	Removes logger setup/configuration guide from the package.

fpfp100 · 2026-02-11T17:24:31Z

@copilot rerun the build

Copilot · 2026-02-11T17:24:41Z

@fpfp100 I've opened a new pull request, #201, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

* fix traceflags is non. * Add test coverage for ParentSpanRef.traceFlags behavior (#193) * Initial plan * Add test coverage for ParentSpanRef.traceFlags behavior Co-authored-by: fpfp100 <126631706+fpfp100@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: fpfp100 <126631706+fpfp100@users.noreply.github.com> * Fix traceFlags fallback to prevent unintentional up-sampling (#194) * Initial plan * Fix traceFlags fallback to avoid unintentional up-sampling Co-authored-by: fpfp100 <126631706+fpfp100@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: fpfp100 <126631706+fpfp100@users.noreply.github.com> * comment --------- Co-authored-by: jsl517 <pefan@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>

* Add OutputScope for output message tracing * comments --------- Co-authored-by: jsl517 <pefan@microsoft.com>

Co-authored-by: Alejandro Dominguez <alejad@microsoft.com>

* Support configuring using new configuration provider * Update packages/agents-a365-observability/src/ObservabilityBuilder.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * comments --------- Co-authored-by: jsl517 <pefan@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…age (#204) * add updateExportToken for mutable per-request token refresh * test update * add unit test --------- Co-authored-by: jsl517 <pefan@microsoft.com>

Co-authored-by: jsl517 <pefan@microsoft.com>

#195) * allow setting PerRequestSpanProcessor without directly exporting the configuration methods at index level. * comments * clean up to use existing class * comment * remove tasks.json * comment * add max age and default grace period * comment * update comment --------- Co-authored-by: jsl517 <pefan@microsoft.com>

* feat: allow start and end time to be set on spans Add setStartTime/setEndTime on OpenTelemetryScope. Forward optional startTime/endTime through all scope subclasses and ScopeUtils. LangChain tracer: use run.start_time/end_time with try/catch resilience. * fix: address PR #205 review feedback - Align license headers to MIT format in scope files - Add @opentelemetry/api dependency to observability-hosting package - Fix test tracer provider setup to reuse existing global provider - Remove trace.disable() in test teardown to avoid breaking other tests - Clamp negative duration to zero in OpenTelemetryScope Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * remove setStartTime since it does not really set the start time since it can only be set through constructor * address additional comments from copilot * log invalid timeinput type * update the lock file --------- Co-authored-by: jsl517 <pefan@microsoft.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Nikhil Navakiran <nikhil.navakiran@gmail.com>

* Support trace context propagation in addition to explicit parent span refs Scope classes now accept a ParentContext (union of ParentSpanRef | Context), enabling W3C traceparent/tracestate header propagation for distributed tracing across HTTP service boundaries while remaining backward compatible with the existing ParentSpanRef approach. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * comment --------- Co-authored-by: jsl517 <pefan@microsoft.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 40 out of 41 changed files in this pull request and generated 7 comments.

Copilot · 2026-02-19T23:19:36Z

tests/observability/core/PerRequestSpanProcessor.test.ts

+      } finally {
+        contextManager.disable();
+        context.disable();
+      }


The finally block disables the global OpenTelemetry context API via context.disable(). This mutates global state for the entire Jest worker and can break subsequent tests that rely on context propagation (including other tests in this file). Instead of disabling the API globally, restore the previous global ContextManager (or set it back to the default one used by the test suite) after the test completes.

Copilot · 2026-02-19T23:19:36Z

packages/agents-a365-observability/src/utils/logging.ts

   * Log an event with standardized parameters
-   * @param eventType Standardized event name/category (e.g., ExporterEventNames.EXPORT)
+   * @param eventType Standardized event name from ExporterEventNames enum (e.g., ExporterEventNames.EXPORT)
   * @param isSuccess Whether the operation/event succeeded
   * @param durationMs Duration of the operation/event in milliseconds
   * @param message Optional message or additional details about the event, especially useful for errors or failures
-   * @param correlationId Optional correlation identifier to connect events/logs across components
-
+   * @param details Optional key-value pairs with additional context (e.g., correlationId, tenantId, agentId, etc.)
   */
-  event(eventType: string, isSuccess: boolean, durationMs: number, message?: string, correlationId?: string): void;
+  event(eventType: ExporterEventNames, isSuccess: boolean, durationMs: number, message?: string, details?: Record<string, string>): void;
 }


The public ILogger.event signature now requires eventType: ExporterEventNames instead of string. This is a breaking TypeScript change for any consumers with existing custom loggers that accept arbitrary strings. Consider widening the type (e.g., string | ExporterEventNames) or keeping string while documenting/encouraging use of ExporterEventNames for low-cardinality events.

Copilot · 2026-02-19T23:19:37Z

packages/agents-a365-observability/src/tracing/context/trace-context-propagation.ts

+ * const headers: Record<string, string> = {};
+ * injectTraceContext(headers);
+ * await fetch('http://service-b/process', { headers });
+ * ```
+ */
+export function injectTraceContext(
+  headers: Record<string, string>,
+  ctx?: Context
+): Record<string, string> {


injectTraceContext is documented as compatible with Node.js IncomingHttpHeaders, but its parameter type is Record<string, string>, which forces callers to cast when they have Record<string, string | string[] | undefined>. Consider accepting HeadersCarrier (or a generic carrier) for headers to match the stated compatibility and the type used by extractTraceContext.

Suggested change

* const headers: Record<string, string> = {};

* injectTraceContext(headers);

* await fetch('http://service-b/process', { headers });

* ```

*/

export function injectTraceContext(

headers: Record<string, string>,

ctx?: Context

): Record<string, string> {

* const headers: HeadersCarrier = {};

* injectTraceContext(headers);

* await fetch('http://service-b/process', { headers });

* ```

*/

export function injectTraceContext(

headers: HeadersCarrier,

ctx?: Context

): HeadersCarrier {

Copilot · 2026-02-19T23:19:37Z

packages/agents-a365-observability/src/tracing/context/parent-span-context.ts

+  // 3. Default to SAMPLED — manually instrumented spans should always be captured
+  const activeCtx = trace.getSpan(base)?.spanContext();
+  const traceFlags =
+    parent.traceFlags
+    ?? (activeCtx?.traceId === parent.traceId ? activeCtx.traceFlags : undefined)
+    ?? TraceFlags.SAMPLED;


createContextWithParentSpanRef now defaults traceFlags to TraceFlags.SAMPLED when the caller doesn't provide parent.traceFlags and there is no matching active trace. This changes behavior from “do not up-sample unless requested” to “always sample”, which can increase exported span volume unexpectedly. If the intent is to avoid surprises, consider keeping the previous default (NONE) and requiring callers to explicitly opt into sampling, or clearly documenting this behavior change as part of the public API contract.

Suggested change

// 3. Default to SAMPLED — manually instrumented spans should always be captured

const activeCtx = trace.getSpan(base)?.spanContext();

const traceFlags =

parent.traceFlags

?? (activeCtx?.traceId === parent.traceId ? activeCtx.traceFlags : undefined)

?? TraceFlags.SAMPLED;

// 3. Default to NONE to avoid up-sampling unless explicitly requested

const activeCtx = trace.getSpan(base)?.spanContext();

const traceFlags =

parent.traceFlags

?? (activeCtx?.traceId === parent.traceId ? activeCtx.traceFlags : undefined)

?? TraceFlags.NONE;

Copilot · 2026-02-19T23:19:37Z

tests/observability/core/token-context.test.ts

+    it('should handle legacy raw string values stored directly in context', () => {
+      const legacyKey = createContextKey('a365_export_token');
+      const ctxWithLegacyToken = context.active().setValue(legacyKey, 'legacy-string-token');
+
+      expect(getExportToken(ctxWithLegacyToken)).toBe('legacy-string-token');
+    });


This test attempts to simulate a legacy token stored under createContextKey('a365_export_token'), but context keys are unique by identity; creating a new key here won’t match the internal EXPORT_TOKEN_KEY used by getExportToken, so the expectation will always fail. To test backward compatibility, the test needs a way to set a raw string value under the actual internal key (e.g., exporting the key for tests, or adding a test-only helper), or the legacy-compat branch should be removed if it can’t be exercised in practice.

Copilot · 2026-02-19T23:19:37Z

tests/observability/core/trace-context-propagation.test.ts

+  beforeAll(() => {
+    contextManager = new AsyncLocalStorageContextManager();
+    contextManager.enable();
+    otelContext.setGlobalContextManager(contextManager);
+    propagation.setGlobalPropagator(new W3CTraceContextPropagator());
+


This test sets a global propagator via propagation.setGlobalPropagator(...) but does not restore the previous propagator in afterAll. Because Jest runs test files in the same process, this can leak global state into other tests and cause order-dependent failures. Save the prior propagator (or reset to the default) in cleanup.

Copilot · 2026-02-19T23:19:38Z

packages/agents-a365-observability/src/ObservabilityBuilder.ts

+  /**
+   * Configures the configuration provider for ObservabilityConfiguration.
+   * When set, this provider is used by the builder and its internal components
+   * instead of the default provider that reads from environment variables.
+   * @param configProvider The configuration provider
+   * @returns The builder instance for method chaining
+   */
+  public withConfigurationProvider(configProvider: IConfigurationProvider<ObservabilityConfiguration>): ObservabilityBuilder {
+    this.options.configProvider = configProvider;
+    return this;
+  }


The PR title/description indicate this change is only removing newly added observability Markdown files, but this diff introduces substantial new public API surface (new scopes, context propagation helpers, configuration-provider plumbing, etc.). Please update the PR title/description to reflect the actual scope so reviewers and release notes can track the behavioral/API changes accurately.

remove newly added md files under observability

16c5a4b

fpfp100 requested a review from a team as a code owner February 9, 2026 19:47

Copilot AI review requested due to automatic review settings February 9, 2026 19:47

Copilot AI reviewed Feb 9, 2026

View reviewed changes

nikhilNava previously approved these changes Feb 10, 2026

View reviewed changes

threddy previously approved these changes Feb 11, 2026

View reviewed changes

juliomenendez previously approved these changes Feb 11, 2026

View reviewed changes

fpfp100 enabled auto-merge (squash) February 11, 2026 17:21

Copilot AI mentioned this pull request Feb 11, 2026

Rerun build after removing observability documentation files #201

Merged

Initial plan (#201)

af69418

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

fpfp100 disabled auto-merge February 19, 2026 23:03

fpfp100 and others added 9 commits February 19, 2026 15:11

Add OutputScope for output message (#199)

6f915e2

* Add OutputScope for output message tracing * comments --------- Co-authored-by: jsl517 <pefan@microsoft.com>

ILogger.event should emit low cardinality event names (#198)

d8865b7

Co-authored-by: Alejandro Dominguez <alejad@microsoft.com>

Improve PerRequestSpanProcessor to allow set export token at later st…

8c6c3a6

…age (#204) * add updateExportToken for mutable per-request token refresh * test update * add unit test --------- Co-authored-by: jsl517 <pefan@microsoft.com>

Map agentBlueprintId and agentUPN in ScopeUtils derive helpers (#203)

b062c79

Co-authored-by: jsl517 <pefan@microsoft.com>

Copilot AI review requested due to automatic review settings February 19, 2026 23:12

fpfp100 dismissed stale reviews from nikhilNava, juliomenendez, and threddy via 43c3ad4 February 19, 2026 23:12

fpfp100 requested a review from a team as a code owner February 19, 2026 23:12

Copilot started reviewing on behalf of fpfp100 February 19, 2026 23:12 View session

fpfp100 closed this Feb 19, 2026

Copilot AI reviewed Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove newly added md files under observability#191

Remove newly added md files under observability#191
fpfp100 wants to merge 11 commits intomainfrom
users/pefan/rmmd

fpfp100 commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

fpfp100 commented Feb 11, 2026

Uh oh!

Copilot AI commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

fpfp100 commented Feb 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

fpfp100 commented Feb 11, 2026

Uh oh!

Copilot AI commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants