-
Notifications
You must be signed in to change notification settings - Fork 290
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
[release] core-1.10.0 release updates #2317
Conversation
@@ -88,7 +87,7 @@ public override void OnEnd(LogRecord data) | |||
|
|||
if (data.Exception != null) | |||
{ | |||
activity.RecordException(data.Exception); | |||
activity.AddException(data.Exception); |
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.
Where something was calling ActivityExtensions.RecordException
I switched it to call Activity.AddException
because Activity.AddException
does the same job (add event matching exception semantic conventions), no migration necessary.
@@ -59,8 +59,10 @@ public override void RecordException(Exception exception, Attributes? attributes | |||
|
|||
var tags = attributes != null ? new TagList(attributes.AllAttributes.ToArray()) : default; | |||
|
|||
this.activity.RecordException(exception, tags); | |||
this.activity.AddException(exception, tags); | |||
#pragma warning disable CS0618 // Type or member is obsolete |
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.
Where something was calling ActivityExtensions.SetStatus
I suppressed warning instead of updating to use Activity.Status
\ Activity.StatusDescription
. The reason for that is they don't do the same things. In order for Activity.Status
\ Activity.StatusDescription
to work, users need to also upgrade to a compatible exporter which understands the status fields on Activity
. I decided calling the old extension for now was the safer bet.
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.
Exception made in ElasticsearchRequestPipelineDiagnosticListener
which was calling the newer Activity APIs to translate http status code but calling the obsolete extensions when there was an exception. Updated to use the newer Activity APIs there so it is consistent.
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.
#2321 created
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.
LGTM
@@ -59,8 +59,10 @@ public override void RecordException(Exception exception, Attributes? attributes | |||
|
|||
var tags = attributes != null ? new TagList(attributes.AllAttributes.ToArray()) : default; | |||
|
|||
this.activity.RecordException(exception, tags); | |||
this.activity.AddException(exception, tags); | |||
#pragma warning disable CS0618 // Type or member is obsolete |
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.
#2321 created
Note: This PR was opened automatically by the core version update workflow.
Merge once packages are available on NuGet and the build passes.
Changes
OpenTelemetryCoreLatestVersion
inCommon.props
to1.10.0
.