Skip to content

Support telemetry processor & initializer #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 21, 2025

Conversation

zhiyuanliang-ms
Copy link
Contributor

@zhiyuanliang-ms zhiyuanliang-ms commented Feb 10, 2025

Why this PR?

This PR is based on the targeting context accessor PR: #93

Support automatically attaching targeting id to telemetry like request, dependency which are collected automatically by app insights

Similar to the exisiting pattern

Usage

Node

const { createTargetingTelemetryProcessor } = require("@microsoft/feature-management-applicationinsights-node");
const myTargetingProcessor = createTargetingTelemetryProcessor(myTargetingContextAccessor);
appInsights.defaultClient.addTelemetryProcessor(myTargetingProcessor);

Browser

const { createTargetingTelemetryInitializer } = require("@microsoft/feature-management-applicationinsights-browser");
const myTargetingInitializer = createTargetingTelemetryInitializer(myTargetingContextAccessor);
appInsights.addTelemetryInitializer(attachTargetingId);

Reference

Classic Application Insights v2 SDK for Node: doc
Application Insights SDK for browser: doc
POC

@zhiyuanliang-ms zhiyuanliang-ms changed the base branch from main to zhiyuanliang/targeting-context-accessor February 10, 2025 11:14
Copy link

@Copilot 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.

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

Files not reviewed (12)
  • src/feature-management-applicationinsights-browser/package.json: Language not supported
  • src/feature-management-applicationinsights-node/package.json: Language not supported
  • src/feature-management/package-lock.json: Language not supported
  • src/feature-management/package.json: Language not supported
  • src/feature-management/src/common/ITargetingContext.ts: Evaluated as low risk
  • src/feature-management/src/filter/TargetingFilter.ts: Evaluated as low risk
  • src/feature-management/src/index.ts: Evaluated as low risk
  • src/feature-management/src/IFeatureManager.ts: Evaluated as low risk
  • src/feature-management-applicationinsights-node/src/index.ts: Evaluated as low risk
  • src/feature-management/src/featureManager.ts: Evaluated as low risk
  • src/feature-management-applicationinsights-browser/src/index.ts: Evaluated as low risk
  • src/feature-management/test/targetingFilter.test.ts: Evaluated as low risk
Comments suppressed due to low confidence (1)

src/feature-management-applicationinsights-node/src/telemetry.ts:52

  • The warning message 'Targeting id is undefined.' is unclear. Consider changing it to 'TargetingContextAccessor returned undefined userId.' to provide more context.
console.warn("Targeting id is undefined.");

@rossgrambo
Copy link

I imagine this PR needs to pull the updates from the other PR

Base automatically changed from zhiyuanliang/targeting-context-accessor to preview March 26, 2025 02:39
rossgrambo
rossgrambo previously approved these changes Apr 17, 2025
CsCherrYY
CsCherrYY previously approved these changes Apr 18, 2025
@zhiyuanliang-ms zhiyuanliang-ms dismissed stale reviews from CsCherrYY and rossgrambo via 325fbef April 21, 2025 07:51
@zhiyuanliang-ms zhiyuanliang-ms merged commit dfbe35e into preview Apr 21, 2025
3 checks passed
@zhiyuanliang-ms zhiyuanliang-ms deleted the zhiyuanliang/telemetry-processor branch April 21, 2025 09:26
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