Skip to content

stdlog observ: remove partial success handling #8174

Merged
dmathieu merged 8 commits intoopen-telemetry:mainfrom
yumosx:feat/yumosx/observ-log
Apr 21, 2026
Merged

stdlog observ: remove partial success handling #8174
dmathieu merged 8 commits intoopen-telemetry:mainfrom
yumosx:feat/yumosx/observ-log

Conversation

@yumosx
Copy link
Copy Markdown
Member

@yumosx yumosx commented Apr 10, 2026

While adding instrumentation for stdoutlog, I found that PartialSuccess handling is not needed for this exporter.
stdoutlog does not return partial-export semantics

yumosx added 2 commits April 10, 2026 14:17
…trumentation tests to accommodate changes in error handling and logging success metrics.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.6%. Comparing base (7529bf0) to head (66934a4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #8174     +/-   ##
=======================================
- Coverage   82.7%   82.6%   -0.1%     
=======================================
  Files        310     309      -1     
  Lines      24648   24621     -27     
=======================================
- Hits       20384   20357     -27     
+ Misses      3888    3887      -1     
- Partials     376     377      +1     
Files with missing lines Coverage Δ
...tdout/stdoutlog/internal/observ/instrumentation.go 97.9% <100.0%> (-0.3%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dashpole dashpole added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 10, 2026
Comment thread exporters/stdout/stdoutlog/internal/observ/instrumentation.go
@flc1125
Copy link
Copy Markdown
Member

flc1125 commented Apr 11, 2026

I need to handle extracting partial successes from err. However, for the metrics part, I still need to accommodate the logic for partial successes or failures. It's just that the source of acquisition is different.

I briefly looked into it, and due to the fact that WithWriter and the use of functions like json.Marshal, there may still be cases of partial success and failure in the following scenarios:

  • When users use a custom Writer, it means they can decide for themselves which parts succeed and which fail.
  • When some of the information passed by the user cannot be properly escaped by json.Marshal, failures may also occur.

@yumosx
Copy link
Copy Markdown
Member Author

yumosx commented Apr 13, 2026

I think in stdlog, we only need to pass the successfully exported value along with the corresponding error when an error occurs—similar to how std trace handles it. The reason for implementing this partial successes error in the first place is that in OTLP we can directly obtain the value associated with the success, whereas in stdlog the situation is different.

in otlplog

if resp != nil && resp.PartialSuccess != nil {
	msg := resp.PartialSuccess.GetErrorMessage()
	n := resp.PartialSuccess.GetRejectedLogRecords()
	if n != 0 || msg != "" {
		err := internal.LogPartialSuccessError(n, msg)
		uploadErr = errors.Join(uploadErr, err)
	}
}

like stdtrace

// Encode span stubs, one by one
if e := e.encoder.Encode(stub); e != nil {
	err = errors.Join(err, fmt.Errorf("failed to encode span %d: %w", i, e))
	continue
}
success++

@yumosx
Copy link
Copy Markdown
Member Author

yumosx commented Apr 15, 2026

I think in stdlog, we only need to pass the successfully exported value along with the corresponding error when an error occurs—similar to how std trace handles it. The reason for implementing this partial successes error in the first place is that in OTLP we can directly obtain the value associated with the success, whereas in stdlog the situation is different.

in otlplog

if resp != nil && resp.PartialSuccess != nil {
	msg := resp.PartialSuccess.GetErrorMessage()
	n := resp.PartialSuccess.GetRejectedLogRecords()
	if n != 0 || msg != "" {
		err := internal.LogPartialSuccessError(n, msg)
		uploadErr = errors.Join(uploadErr, err)
	}
}

like stdtrace

// Encode span stubs, one by one
if e := e.encoder.Encode(stub); e != nil {
	err = errors.Join(err, fmt.Errorf("failed to encode span %d: %w", i, e))
	continue
}
success++

@dashpole I think we should keep consistent with stdtrace's instrument, what do you think?

@dashpole
Copy link
Copy Markdown
Contributor

for _, record := range records {
// Honor context cancellation.
if err := ctx.Err(); err != nil {
return err
}
// Encode record, one by one.
recordJSON := e.newRecordJSON(record)
if err := enc.Encode(recordJSON); err != nil {
return err
}
}

We don't currently support partial success in stdoutlog. If we want to match the behavior of stdouttrace, we should support partial sucecss like it does:

for i := range stubs {
stub := &stubs[i]
// Remove timestamps
if !e.timestamps {
stub.StartTime = zeroTime
stub.EndTime = zeroTime
for j := range stub.Events {
ev := &stub.Events[j]
ev.Time = zeroTime
}
}
// Encode span stubs, one by one
if e := e.encoder.Encode(stub); e != nil {
err = errors.Join(err, fmt.Errorf("failed to encode span %d: %w", i, e))
continue
}
success++
}

@yumosx
Copy link
Copy Markdown
Member Author

yumosx commented Apr 17, 2026

for _, record := range records {
// Honor context cancellation.
if err := ctx.Err(); err != nil {
return err
}
// Encode record, one by one.
recordJSON := e.newRecordJSON(record)
if err := enc.Encode(recordJSON); err != nil {
return err
}
}

We don't currently support partial success in stdoutlog. If we want to match the behavior of stdouttrace, we should support partial sucecss like it does:

for i := range stubs {
stub := &stubs[i]
// Remove timestamps
if !e.timestamps {
stub.StartTime = zeroTime
stub.EndTime = zeroTime
for j := range stub.Events {
ev := &stub.Events[j]
ev.Time = zeroTime
}
}
// Encode span stubs, one by one
if e := e.encoder.Encode(stub); e != nil {
err = errors.Join(err, fmt.Errorf("failed to encode span %d: %w", i, e))
continue
}
success++
}

Yes, that's exactly what I mean.

So in this PR, I removed the partial success approach and instead adopted an instrumentation API style similar to stdtrace.

@dashpole
Copy link
Copy Markdown
Contributor

I see. I'm OK with this as a first step.

@dmathieu dmathieu merged commit 63ee0c5 into open-telemetry:main Apr 21, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants