Skip to content
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

[WIP] POC CustomEvents #48378

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

[WIP] POC CustomEvents #48378

wants to merge 7 commits into from

Conversation

TimothyMothra
Copy link
Contributor

@TimothyMothra TimothyMothra commented Feb 20, 2025

Opening this Draft PR to collect feedback.
If we're good with this overall approach, I need to finish updating all the tests that relied on the old behavior.

Before

In the current implementation, LogsHelper.OtelToAzureMonitorLogs inspects a few top-level properties and then decides if a telemetry item is an AI-Trace or Exception. Then the Data classes will call LogsHelper.GetMessageAndSetProperties to parse all LogRecord Attributes.

After

Moving forward, we need to evaluate LogRecord.Attributes earlier so we can determine if the telemetry item is a Custom Event.

  • LogsHelper.OtelToAzureMonitorLogs was refactored.
  • LogsHelper.GetMessageAndSetProperties was changed to LogsHelper.ProcessLogRecordProperties.

@github-actions github-actions bot added the Monitor - Exporter Monitor OpenTelemetry Exporter label Feb 20, 2025
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

var properties = new ChangeTrackingDictionary<string, string>();
ProcessLogRecordProperties(logRecord, properties, out string? message, out string? eventName);

if (message is not null && logRecord.Exception is not null)
Copy link
Contributor

Choose a reason for hiding this comment

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

If logRecord is exception we don't need to have additional check on CustomEventAttributeName and could use the existing implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monitor - Exporter Monitor OpenTelemetry Exporter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants