Skip to content

Add otel.sdk.processor.log.processed metric#7486

Open
cijothomas wants to merge 8 commits into
open-telemetry:mainfrom
cijothomas:cijothomas/sdk-self-obs-processor-log-processed
Open

Add otel.sdk.processor.log.processed metric#7486
cijothomas wants to merge 8 commits into
open-telemetry:mainfrom
cijothomas:cijothomas/sdk-self-obs-processor-log-processed

Conversation

@cijothomas

@cijothomas cijothomas commented Jul 3, 2026

Copy link
Copy Markdown
Member

Towards #3880

Adds the otel.sdk.processor.log.processed counter metric to BatchLogRecordExportProcessor and SimpleLogRecordExportProcessor, following the OTel SDK self-observability semantic conventions.

This is the first SDK self-observability metric in the .NET SDK. More metrics (queue size, exporter counters, etc.) will follow in subsequent PRs.

Design

  • Meter: otel.sdk.experimental - requires explicit opt-in via .AddMeter("otel.sdk.experimental"). Without this, Counter.Add() is a no-op with zero overhead.
  • Instrument: Counter<long> named otel.sdk.processor.log.processed
  • Attributes:
    • otel.component.type - batching_log_processor or simple_log_processor
    • otel.component.name - unique per instance (e.g. batching_log_processor/0)
    • error.type (conditional) - queue_full when buffer rejects, already_shutdown when processor has been shut down

Opt-in example

var meterProvider = Sdk.CreateMeterProviderBuilder()
    .AddMeter("otel.sdk.experimental")
    .AddOtlpExporter()
    .Build();

Why .AddMeter() instead of env variable or compile-time flag?

Metrics in .NET are inherently opt-in: without a MeterProvider subscribing to the meter, Counter.Add() is a no-op at near-zero cost. The .AddMeter("otel.sdk.experimental") call provides natural gating without requiring additional mechanisms like env variables or feature flags.

Implement the otel.sdk.processor.log.processed counter metric for
BatchLogRecordExportProcessor and SimpleLogRecordExportProcessor.

The metric is emitted on meter 'otel.sdk.experimental' which requires
explicit opt-in via .AddMeter("otel.sdk.experimental") — zero overhead
without a listener.

Attributes:
- otel.component.type: batching_log_processor | simple_log_processor
- otel.component.name: unique instance identifier (e.g. batching_log_processor/0)
- error.type (conditional): queue_full | already_shutdown

Design:
- SdkSelfObservability internal helper owns the Meter and Counter
- Pre-allocated KeyValuePair[] tag arrays (no per-call allocation)
- Batch processor uses TryExport return value to classify success vs queue_full
- Both processors detect post-shutdown state via OnShutdown override
@github-actions github-actions Bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Jul 3, 2026
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.16129% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.23%. Comparing base (8592991) to head (2a53a4a).
⚠️ Report is 26 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/OpenTelemetry/Internal/MeterFactory.cs 80.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7486      +/-   ##
==========================================
+ Coverage   90.21%   90.23%   +0.01%     
==========================================
  Files         287      289       +2     
  Lines       15560    15620      +60     
==========================================
+ Hits        14038    14094      +56     
- Misses       1522     1526       +4     
Flag Coverage Δ
unittests-Project-Experimental 90.26% <95.16%> (+<0.01%) ⬆️
unittests-Project-Stable 90.21% <95.16%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/OpenTelemetry/Internal/SdkSelfObservability.cs 100.00% <100.00%> (ø)
...ry/Logs/Processor/BatchLogRecordExportProcessor.cs 100.00% <100.00%> (+11.11%) ⬆️
...y/Logs/Processor/SimpleLogRecordExportProcessor.cs 100.00% <100.00%> (ø)
src/OpenTelemetry/Internal/MeterFactory.cs 80.00% <80.00%> (ø)

... and 8 files with indirect coverage changes

@cijothomas cijothomas moved this from Todo to In progress in OTel Clients Self Observability Jul 3, 2026
The test was flaky on Windows ARM / net462 because with maxQueueSize=1
and maxExportBatchSize=1, the worker thread could drain the queue faster
than items were being emitted, resulting in no drops. Use a blocking
DelegatingExporter to hold the worker in Export() while filling the queue.
@cijothomas
cijothomas marked this pull request as ready for review July 3, 2026 15:27
@cijothomas
cijothomas requested a review from a team as a code owner July 3, 2026 15:27
Comment thread src/OpenTelemetry/Internal/SdkSelfObservability.cs Outdated
Comment thread src/OpenTelemetry/Internal/SdkSelfObservability.cs Outdated
Comment thread src/OpenTelemetry/Logs/Processor/BatchLogRecordExportProcessor.cs Outdated
break;
}

// TODO: Consider switching to an ObservableCounter that piggybacks on

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.

Issue for tracking?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will file a tracking issue for this.

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.

Cool - just add a link to the TODO once created.

Comment thread src/OpenTelemetry/Logs/Processor/BatchLogRecordExportProcessor.cs Outdated
Comment thread src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt
Comment thread src/OpenTelemetry/Logs/Processor/SimpleLogRecordExportProcessor.cs Outdated
Comment thread src/OpenTelemetry/README.md Outdated
@cijothomas
cijothomas force-pushed the cijothomas/sdk-self-obs-processor-log-processed branch from c6a9a18 to 8c89abe Compare July 4, 2026 01:08
@github-actions github-actions Bot added the infra Infra work - CI/CD, code coverage, linters label Jul 4, 2026
- Copy MeterFactory from contrib for auto-versioning
- Make Meter internal (removes SA1202 suppression)
- Share base tags between success/error arrays
- Single Counter.Add() call with local tag assignment
- Shorten README overhead note
@cijothomas
cijothomas force-pushed the cijothomas/sdk-self-obs-processor-log-processed branch from 8c89abe to 6c5df34 Compare July 4, 2026 01:13
@github-actions github-actions Bot removed the infra Infra work - CI/CD, code coverage, linters label Jul 4, 2026
/// </summary>
public class BatchLogRecordExportProcessor : BatchExportProcessor<LogRecord>
{
private static int instanceCounter = -1;

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.

While unlikely there'd ever be that many created in the lifetime of a process, maybe use long to give more headroom against overflow?

break;
}

// TODO: Consider switching to an ObservableCounter that piggybacks on

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.

Cool - just add a link to the TODO once created.

@github-actions

Copy link
Copy Markdown
Contributor

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions Bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Jul 12, 2026
@Kielek Kielek removed the Stale Issues and pull requests which have been flagged for closing due to inactivity label Jul 13, 2026
@Kielek

Kielek commented Jul 13, 2026

Copy link
Copy Markdown
Member

@cijothomas, I asked Codex /GPT 5.6 Sol. Briefly checked findings and it looks valid to me:

Following comment is AI driven


Reviewed commit: 2a53a4a

1. Count batch success at exporter submission, not queue admission

Priority: P1

BatchLogRecordExportProcessor.cs:108
increments the success counter immediately after TryExport enqueues the log
record.

The semantic convention says that, for simple and batching processors, a log
record is considered processed when it has been submitted to the exporter, not
when the corresponding export call finishes. For the batching processor,
submission happens later on the worker thread. The current implementation can
therefore report records as successfully processed while they are still queued.
If draining does not complete, some of those records may never be submitted to
the exporter.

The successful increment should happen at the worker/export boundary. Queue
rejection failures can continue to be recorded where enqueueing is attempted.
The BatchProcessor_SuccessfulEnqueue test should also verify that success is
not recorded before the exporter receives the record.

Specification:
https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/#metric-otelsdkprocessorlogprocessed

2. The simple processor reports exported records as already_shutdown

Priority: P1

SimpleLogRecordExportProcessor.cs:38-42
calls base.OnEnd(data) before checking isShutdown.

SimpleExportProcessor does not prevent OnEnd from invoking
Exporter.Export after Shutdown() has been called. As a result, a record
emitted after shutdown is still submitted to the exporter, but the new metric
labels it with error.type=already_shutdown. The README describes that record
as lost, although it was exported.

The processor should either reject records after shutdown before invoking the
exporter, or report the successful submission that actually occurred. The
shutdown test should retain the exporter's collection and assert whether the
post-shutdown record was exported.

The semantic convention reserves already_shutdown for records dropped because
the processor has already been shut down:
https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/#metric-otelsdkprocessorlogprocessed

3. Shutdown races can mislabel successfully queued batch records

Priority: P2

BatchLogRecordExportProcessor.cs:95-108
reads isShutdown after attempting to enqueue the record.

For example, the following interleaving is possible:

  1. OnEnd successfully enqueues a record.
  2. A concurrent Shutdown() sets isShutdown and includes the record in its
    drain target.
  3. OnEnd observes isShutdown and records already_shutdown.
  4. The worker submits the record to the exporter during shutdown draining.

This reports a successfully processed record as a shutdown failure. Acceptance
and shutdown need an atomic outcome, rather than classifying the result using a
separate volatile flag after enqueueing.

4. Add the required SDK CHANGELOG entry

Priority: P2

This is a behavioral addition to the OpenTelemetry component, but the changed
files do not include src/OpenTelemetry/CHANGELOG.md. Repository policy requires
an entry at the end of that file's Unreleased section, using PR #7486 in the
link.

{
using (var batch = new Batch<T>(this.CircularBuffer, this.MaxExportBatchSize))
{
this.exportStarted?.Invoke(batch.Count);

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.

Should we defend against this callback throwing so that Export() is still called?

Comment on lines +122 to +126
SpinWait spinner = default;
while (Volatile.Read(ref this.activeOnEndCount) != 0)
{
spinner.SpinOnce();
}

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.

Should this honour timeoutMilliseconds too?

Comment on lines +70 to +71
SpinWait spinner = default;
while (Volatile.Read(ref this.activeOnEndCount) != 0)

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.

Same here.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 18, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-24 08:59:23 UTC.

  • Waiting on: Author
  • Next step: Address or respond to 4 review feedback items:
    • Inline threads: 1, 2, 3, 4
    • For each item, reply to move the discussion forward, e.g. link to the commit that addresses it, explain why no change is needed, or ask a follow-up question.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers. If the last refreshed time above predates your latest reply or push, the dashboard hasn't processed it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

3 participants