Skip to content

Conversation

@mpodwysocki
Copy link
Contributor

Packages impacted by this PR

  • @azure/opentelemetry-instrumentation-azure-sdk

Issues associated with this PR

Describe the problem that is addressed by this PR

Updates to latest OTEL

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

Are there test cases added in this PR? (If not, why?)

Provide a list of related PRs (if any)

Command used to generate this PR:**(Applicable only to SDK release request PRs)

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

// Function syntax
instrumenter.withContext(context.active(), function (this: any) {
assert.notExists(this);
assert.isDefined(this);
Copy link
Member

Choose a reason for hiding this comment

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

This seems weird to me, but I don't want to block the PR. Feel free to pend the test and assign an issue to me to review it more closely. It's been like 2 years 😄

Copy link
Member

Choose a reason for hiding this comment

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

yeah feels like it should be undefined as it's not being explicitly bound to anything?

Copy link
Member

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for taking care of this

// Function syntax
instrumenter.withContext(context.active(), function (this: any) {
assert.notExists(this);
assert.isDefined(this);
Copy link
Member

Choose a reason for hiding this comment

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

yeah feels like it should be undefined as it's not being explicitly bound to anything?

@xirzec
Copy link
Member

xirzec commented Mar 11, 2024

/check-enforcer reset

@xirzec
Copy link
Member

xirzec commented Mar 11, 2024

/check-enforcer evaluate

@xirzec
Copy link
Member

xirzec commented Mar 11, 2024

@microsoft-github-policy-service rerun

@xirzec
Copy link
Member

xirzec commented Mar 11, 2024

/check-enforcer reset

@xirzec
Copy link
Member

xirzec commented Mar 11, 2024

/check-enforcer evaluate

@mpodwysocki mpodwysocki merged commit 15f13e9 into main Mar 11, 2024
@mpodwysocki mpodwysocki deleted the feat/otel-monitor-instrument branch March 11, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants