Skip to content
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/6.0-preview5] Regression and Perf Fixes #52983

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 19, 2021

Backport of #52956 to release/6.0-preview5

/cc @tarekgh

Customer Impact

The changes here are addressing some regression causing some failures in the OpenTelemetery when started using .NET 6.0 bits. Also, it fixes some memory allocation issue in the new exposed Metrics APIs for preview 5.

here is more info about the changes:

  • Fixes a regression of setting the recording trace flags on the Activity when having a parent context and the samplers returning a recording flag on. We used to use the parent context trace flags but now we consider the sampler returned value.
  • Fixes extra allocation which was happening when recording a measurement in the metrics instruments. The original written code was working fine on .NET 5.0 and 6.0 as the JIT was avoiding the extra allocation. But on .NET Core and Full Framework this optimization was not happening. The change here is to avoid this extra allocation.

Testing

This has been tested with the OpenTelemetry failures and ensured fixing that. Also, added more tests for the failures in our regression tests. We have validated the memory allocation issue is fixed by manually instrumenting the scenario and looking at the allocated memory.

Risk

Is very low. The reason is, part of the change didn't change anything more than moving a couple of source lines a few lines down and we successfully run all tests against the change too. The other part of the change is in the newly exposed Metrics APIs which is going to have the first release in preview 5. In addition, the change is simple and not expected to cause any issue.

@ghost
Copy link

ghost commented May 19, 2021

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #52956 to release/6.0-preview5

/cc @tarekgh

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Diagnostics.Tracing

Milestone: -

@tarekgh tarekgh self-assigned this May 19, 2021
@tarekgh tarekgh added this to the 6.0.0 milestone May 19, 2021
@tarekgh tarekgh added the Servicing-consider Issue for next servicing release review label May 19, 2021
@tarekgh tarekgh requested a review from noahfalk May 19, 2021 18:22
@tarekgh
Copy link
Member

tarekgh commented May 19, 2021

@ericstj @Anipik this is backport request to preview 5. Please let me know if there is anything else I need to do.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Looks great : )

@ericstj ericstj added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels May 21, 2021
@ericstj ericstj merged commit 691c12b into release/6.0-preview5 May 21, 2021
@jkotas jkotas deleted the backport/pr-52956-to-release/6.0-preview5 branch May 21, 2021 03:26
@ghost ghost locked as resolved and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants