Skip to content

[LiveMetrics] add support for collecting logs#43112

Closed
TimothyMothra wants to merge 2 commits intotilee/livemetrics_refactor_documentfrom
tilee/livemetrics_log2_withServiceCollection
Closed

[LiveMetrics] add support for collecting logs#43112
TimothyMothra wants to merge 2 commits intotilee/livemetrics_refactor_documentfrom
tilee/livemetrics_log2_withServiceCollection

Conversation

@TimothyMothra
Copy link
Copy Markdown

Supersedes #42889
Follow up to #43081. (MUST wait for 43081 to merge before we can merge THIS pr)

This PR adds functionality to collect logs and send to LiveMetrics as TraceDocuments.

Changes

  • Add new class LiveMetricsLogProcessor that collects OpenTelemetry LogRecord
  • update LiveMetricsExtensions
    • signal specific extension methods are changed to private
    • new public extension method for OpenTelemetryBuilder. This depends on IServiceCollection to manage internal singletons.
  • update Demo project
    • Until this project is rewritten as an AspNetCore application, had to manually setup a ServiceCollection

<PackageReference Include="Azure.Core" />
<PackageReference Include="OpenTelemetry" />
<PackageReference Include="OpenTelemetry.Exporter.Console" VersionOverride="1.6.0" />
<PackageReference Include="OpenTelemetry.Extensions.Hosting" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't we achieve this feature without this reference?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It could, but I don't think we want to.

This PR delivers a single extension method that configures LiveMetrics for both signals.
This extension method uses OpenTelemetryBuilder which comes from OpenTelemetry.Extensions.Hosting.

public static OpenTelemetryBuilder AddLiveMetrics(this OpenTelemetryBuilder builder, Action<LiveMetricsExporterOptions> configure = null)

Without this package, a user would need to configure our Manager singleton to the ServiceCollection and set LiveMetrics on both signals. I don't think we should take this approach.

Second, as we're preparing to shift our implementation to the Distro, it makes sense to take common dependencies now. This will make that shift easier.

Copy link
Copy Markdown
Member

@rajkumar-rangaraj rajkumar-rangaraj left a comment

Choose a reason for hiding this comment

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

The direction of this PR, which involves exposing a new API, is not the correct approach.

@TimothyMothra TimothyMothra marked this pull request as draft April 11, 2024 20:19
@TimothyMothra
Copy link
Copy Markdown
Author

The direction of this PR, which involves exposing a new API, is not the correct approach.

We should sync offline, I think a new API is unavoidable until this code moves to the Distro project.

The way I see it we have two options. For context, as of today we have one public extension method for TracerProviderBuilder.

  1. Replace the current method with a single extension method for OpenTelemetryBuilder that configures LiveMetrics for both Traces and Logs.
  2. Add a second extension method for OpenTelemetryLoggerOptions.

Without either of these, our Distro project cannot setup LiveMetrics.

@rajkumar-rangaraj
Copy link
Copy Markdown
Member

The direction of this PR, which involves exposing a new API, is not the correct approach.

We should sync offline, I think a new API is unavoidable until this code moves to the Distro project.

The way I see it we have two options. For context, as of today we have one public extension method for TracerProviderBuilder.

  1. Replace the current method with a single extension method for OpenTelemetryBuilder that configures LiveMetrics for both Traces and Logs.
  2. Add a second extension method for OpenTelemetryLoggerOptions.

Without either of these, our Distro project cannot setup LiveMetrics.

I'm working on this foundation, will send a PR shortly.

@jsquire jsquire deleted the tilee/livemetrics_log2_withServiceCollection branch February 18, 2025 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants