error.type for wrapped errors

[Balaji01-4D]

Fixes #7975

When the reflection fallback derives error.type, walk the standard Go error chain using errors.Unwrap() and reflect on the final unwrapped error instead of the outer wrapper fmt.wrapError.

Scope:

• Only modifies the reflection fallback logic.
• Aggregated errors (errors.Join) are unchanged.

[Copilot]

Pull request overview

Updates the semconv v1.40.0 ErrorType reflection fallback to avoid reporting wrapper types (notably *fmt.wrapError) by unwrapping via errors.Unwrap() before reflecting, and extends the generated tests/templates accordingly.

Changes:

• Walk the standard Go unwrap chain in the reflection fallback and reflect on the final unwrapped error.
• Adjust semconv/v1.40.0 tests to validate behavior for fmt.Errorf("%w", ...) wrappers and custom wrappers.
• Mirror the logic and test updates in semconvkit templates so future generated semconv packages match.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
semconv/v1.40.0/error_type.go	Changes reflection fallback to unwrap the error chain before deriving error.type.
semconv/v1.40.0/error_type_test.go	Adds coverage for wrapped errors and updates expected type selection behavior.
internal/tools/semconvkit/templates/error_type.go.tmpl	Mirrors the unwrap-before-reflect logic in the code generator template.
internal/tools/semconvkit/templates/error_type_test.go.tmpl	Mirrors the updated test expectations in the generator template.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pellared]

Can you please add a changelog entries under a Changed section?

[pellared]

Please apply the same change here:

opentelemetry-go/sdk/log/logger.go

Lines 160 to 185 in 4b025b4

	func errorType(err error) string {
	if et, ok := err.(interface{ ErrorType() string }); ok {
	if s := et.ErrorType(); s != "" {
	return s
	}
	}
	t := reflect.TypeOf(err)
	if t == nil {
	return ""
	}
	pkg, name := t.PkgPath(), t.Name()
	if pkg != "" && name != "" {
	return pkg + "." + name
	}
	// The type has no package path or name (predeclared, not-defined,
	// or alias for a not-defined type).
	//
	// The type has no package path or name (predeclared, not-defined,
	// or alias for a not-defined type).
	//
	// This is not guaranteed to be unique, but is a best effort.
	return t.String()
	}

[Balaji01-4D]

Please apply the same change here:

opentelemetry-go/sdk/log/logger.go

Lines 160 to 185 in 4b025b4

	func errorType(err error) string {
	if et, ok := err.(interface{ ErrorType() string }); ok {
	if s := et.ErrorType(); s != "" {
	return s
	}
	}
	t := reflect.TypeOf(err)
	if t == nil {
	return ""
	}
	pkg, name := t.PkgPath(), t.Name()
	if pkg != "" && name != "" {
	return pkg + "." + name
	}
	// The type has no package path or name (predeclared, not-defined,
	// or alias for a not-defined type).
	//
	// The type has no package path or name (predeclared, not-defined,
	// or alias for a not-defined type).
	//
	// This is not guaranteed to be unique, but is a best effort.
	return t.String()
	}

I noticed that PR #7994 also modified the ErrorType function in internal/tools/semconvkit/templates/error_type.go.tmpl. Should those changes be considered here as well?

[pellared]

Please apply the same change here:

opentelemetry-go/sdk/log/logger.go

Lines 160 to 185 in 4b025b4

	func errorType(err error) string {
	if et, ok := err.(interface{ ErrorType() string }); ok {
	if s := et.ErrorType(); s != "" {
	return s
	}
	}
	t := reflect.TypeOf(err)
	if t == nil {
	return ""
	}
	pkg, name := t.PkgPath(), t.Name()
	if pkg != "" && name != "" {
	return pkg + "." + name
	}
	// The type has no package path or name (predeclared, not-defined,
	// or alias for a not-defined type).
	//
	// The type has no package path or name (predeclared, not-defined,
	// or alias for a not-defined type).
	//
	// This is not guaranteed to be unique, but is a best effort.
	return t.String()
	}

I noticed that PR #7994 also modified the ErrorType function in internal/tools/semconvkit/templates/error_type.go.tmpl. Should those changes be considered here as well?

This would be great!

[Balaji01-4D]

@pellared I have completed both changes. I applied the same change in sdk/log/logger.go and added test cases for TestErrorType covering wrapped and joined errors.

[pellared]

@seh, @MrAlias, I would appreciate your reviews/approvals.

[codecov]

Codecov Report

❌ Patch coverage is 70.96774% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.5%. Comparing base (2ffde5a) to head (c9b7ec0).

Files with missing lines	Patch %	Lines
semconv/v1.40.0/error_type.go	0.0%	8 Missing ⚠️
sdk/log/logger.go	95.6%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #8018     +/-   ##
=======================================
- Coverage   81.6%   81.5%   -0.1%     
=======================================
  Files        304     304             
  Lines      23452   23473     +21     
=======================================
+ Hits       19141   19150      +9     
- Misses      3928    3939     +11     
- Partials     383     384      +1     

Files with missing lines	Coverage Δ
sdk/log/logger.go	98.9% <95.6%> (-1.1%)	⬇️
semconv/v1.40.0/error_type.go	0.0% <0.0%> (ø)

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

[Balaji01-4D]

Done add740f

@pellared , Thanks. I will work on the errors.Join part once a dedicated issue is opened.

[seh]

I will work on the errors.Join part once a dedicated issue is opened.

Speaking of errors.Join, did you see my #7856 (comment) over on #7856?

[Balaji01-4D]

Speaking of errors.Join, did you see my #7856 (comment) over on #7856?

Thanks for pointing out. I saw your comment on #7856. You are right that the implementation builds a nested chain of errors.joinError instead of a flat sequence of errors. I’ll take note of this.

[seh]

This replaces the current reporting solely of the outermost error with reporting solely of the innermost. There are three types that could be involved potentially, though, for these errors:

• The outermost error
• The error returned by the first call of errors.Unwrap (presumably non-nil) on the outermost error
• The innermost error discovered by digging through successive calls to errors.Unwrap, as you've done here

This change would improve things from my perspective, but I'm anticipating that others will complain that they're interested more in one of the first two types mentioned above.

Do we need a semantic convention to bless us introducing a separate attribute name to report on these types—especially the second two—when they differ?

[Balaji01-4D]

Good point. Using rootError changes the behavior from reporting the outermost error to solely reporting the innermost one discovered via repeated errors.Unwrap. I agree this could blur the distinction between the outermost error and the first Unwrap result.

My reasoning was that the innermost error typically represents the concrete failure type, while outer wrappers often add context as mention this comment, but not meant for categorize as you noted.

If distinguishing those cases is important, introducing a separate attribute for wrapper types might make the semantics clearer rather than overwriting error.type. for example, error.cause.type for the first Unwrap result and likewise error.root.type. However, I’m not entirely sure about this.

[seh]

Note that this test and the next three would fail if the standard library changed the names of these two unexported types.

[Balaji01-4D]

I see. These tests rely on unexported stdlib name which could change. Would it make sense to avoid asserting the exact type string?

[seh]

Mentioning "unwraps" is correct mechanically, but it doesn't clarify how much unwrapping we're now doing. Consider including "as deeply as possible" or "as completely as possible" after "error chains".