-
Couldn't load subscription status.
- Fork 2
fix: Ensure additional attributes are included in all cases. #264
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
base: main
Are you sure you want to change the base?
Conversation
| span.AddEvent(semconv.ExceptionEventName, trace.WithAttributes(exceptionAttributes...)) | ||
| } else { | ||
| span.RecordError(err, trace.WithStackTrace(true)) | ||
| span.RecordError(err, trace.WithStackTrace(true), trace.WithAttributes(tags...)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attributes were not forwarded here.
|
|
||
| // RecordLog is used to record arbitrary logs in your golang backend. | ||
| func RecordLog(ctx context.Context, record log.Record, tags ...log.KeyValue) error { | ||
| record.AddAttributes(tags...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theoretically these could already be in the record instead. How would we feel about removing tags from the parameters and just requiring they be in the record? It would be a breaking change, but we have not gone 1.0 and it would be minor to adjust for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds like a good change to me. IIRC the public RecordLog api is a relatively new addition
|
|
||
| // RecordLog is used to record arbitrary logs in your golang backend. | ||
| func RecordLog(ctx context.Context, record log.Record, tags ...log.KeyValue) error { | ||
| record.AddAttributes(tags...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds like a good change to me. IIRC the public RecordLog api is a relatively new addition
Summary
How did you test this change?
Are there any deployment considerations?
Note
Ensures supplied tags are included on exception events, span.RecordError calls, and log records.
tagsonspan.RecordError(..., trace.WithAttributes(tags...))when no stack trace is present.exceptionAttributesand append providedtagsbeforespan.AddEvent.tagstolog.Recordviarecord.AddAttributes(tags...)before emitting.Written by Cursor Bugbot for commit 5f3ec5c. This will update automatically on new commits. Configure here.