Skip to content

Conversation

@sourabh1007
Copy link
Contributor

@sourabh1007 sourabh1007 commented Oct 9, 2024

Description

As part of this PR, adding back attributes required by appinsights sdk. It broke the customer experience with appinsight as Appinsight SDK supports very specific set of attributes.

This implementation will change in future release, where open telemetry attributes will be controlled by Env Variable as mentioned here.
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#semantic-conventions-for-database-client-calls

4.42.3
image

v4.43.0
image

v4.43.1
image

After this PR:
image

After this change, customer has to set OTEL_SEMCONV_STABILITY_OPT_IN to database/dup, in order to see latest attributes. otherwise SDK will emit only classic attributes which would be compatible with appinsights sdk also.

Type of change

  • [] Bug fix (non-breaking change which fixes an issue)

Closing issues

To automatically close an issue: closes #IssueNumber

Copy link
Contributor

@philipthomas-MSFT philipthomas-MSFT left a comment

Choose a reason for hiding this comment

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

LGTM. I will re-review after Draft version

@sourabh1007 sourabh1007 added the auto-merge Enables automation to merge PRs label Oct 9, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot enabled auto-merge (squash) October 9, 2024 17:12
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit 398d22c into master Oct 9, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the users/sourabhjain/bugfix branch October 9, 2024 17:13
sourabh1007 added a commit that referenced this pull request Oct 14, 2024
…nsights sdk (#4781)

## Description

As part of this PR, adding back attributes required by appinsights sdk.
It broke the customer experience with appinsight as Appinsight SDK
supports very specific set of attributes.

This implementation will change in future release, where open telemetry
attributes will be controlled by Env Variable as mentioned here.

https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#semantic-conventions-for-database-client-calls

**4.42.3**

![image](https://github.com/user-attachments/assets/6c19eab0-4884-4648-b6d9-c941d58e8e0b)

**v4.43.0**

![image](https://github.com/user-attachments/assets/fd2a70b4-ed37-4618-b1a5-f654587bf6ab)

**v4.43.1**

![image](https://github.com/user-attachments/assets/5bbe54ec-34f3-4538-a7b4-abff1b1853ea)

After this PR:

![image](https://github.com/user-attachments/assets/2ea64a34-4e57-4dec-a29e-32ffcd77919a)

After this change, customer has to set `OTEL_SEMCONV_STABILITY_OPT_IN`
to `database/dup`, in order to see latest attributes. otherwise SDK will
emit only classic attributes which would be compatible with appinsights
sdk also.

## Type of change
- [] Bug fix (non-breaking change which fixes an issue)

## Closing issues

To automatically close an issue: closes #IssueNumber
microsoft-github-policy-service bot pushed a commit that referenced this pull request Oct 15, 2024
…support appinsights sdk (#4781) (#4799)

## Description

As part of this PR, adding back attributes required by appinsights sdk.
It broke the customer experience with appinsight as Appinsight SDK
supports very specific set of attributes.

This implementation will change in future release, where open telemetry
attributes will be controlled by Env Variable as mentioned here.


https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#semantic-conventions-for-database-client-calls

**4.42.3**


![image](https://github.com/user-attachments/assets/6c19eab0-4884-4648-b6d9-c941d58e8e0b)

**v4.43.0**


![image](https://github.com/user-attachments/assets/fd2a70b4-ed37-4618-b1a5-f654587bf6ab)

**v4.43.1**


![image](https://github.com/user-attachments/assets/5bbe54ec-34f3-4538-a7b4-abff1b1853ea)

After this PR:


![image](https://github.com/user-attachments/assets/2ea64a34-4e57-4dec-a29e-32ffcd77919a)

After this change, customer has to set `OTEL_SEMCONV_STABILITY_OPT_IN`
to `database/dup`, in order to see the otel (new and old) attributes.
otherwise SDK will emit only classic attributes which would be
compatible with appinsights sdk also.

## Type of change
- [] Bug fix (non-breaking change which fixes an issue)
sourabh1007 added a commit that referenced this pull request Oct 15, 2024
…support appinsights sdk (#4781) (#4799)

## Description

As part of this PR, adding back attributes required by appinsights sdk.
It broke the customer experience with appinsight as Appinsight SDK
supports very specific set of attributes.

This implementation will change in future release, where open telemetry
attributes will be controlled by Env Variable as mentioned here.


https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#semantic-conventions-for-database-client-calls

**4.42.3**


![image](https://github.com/user-attachments/assets/6c19eab0-4884-4648-b6d9-c941d58e8e0b)

**v4.43.0**


![image](https://github.com/user-attachments/assets/fd2a70b4-ed37-4618-b1a5-f654587bf6ab)

**v4.43.1**


![image](https://github.com/user-attachments/assets/5bbe54ec-34f3-4538-a7b4-abff1b1853ea)

After this PR:


![image](https://github.com/user-attachments/assets/2ea64a34-4e57-4dec-a29e-32ffcd77919a)

After this change, customer has to set `OTEL_SEMCONV_STABILITY_OPT_IN`
to `database/dup`, in order to see the otel (new and old) attributes.
otherwise SDK will emit only classic attributes which would be
compatible with appinsights sdk also.

## Type of change
- [] Bug fix (non-breaking change which fixes an issue)
kirankumarkolli added a commit that referenced this pull request Oct 15, 2024
…support appinsights sdk (#4781) (#4799) (#4804)

## Description

As part of this PR, adding back attributes required by appinsights sdk.
It broke the customer experience with appinsight as Appinsight SDK
supports very specific set of attributes.

This implementation will change in future release, where open telemetry
attributes will be controlled by Env Variable as mentioned here.



https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-spans.md#semantic-conventions-for-database-client-calls

**4.42.3**



![image](https://github.com/user-attachments/assets/6c19eab0-4884-4648-b6d9-c941d58e8e0b)

**v4.43.0**



![image](https://github.com/user-attachments/assets/fd2a70b4-ed37-4618-b1a5-f654587bf6ab)

**v4.43.1**



![image](https://github.com/user-attachments/assets/5bbe54ec-34f3-4538-a7b4-abff1b1853ea)

After this PR:



![image](https://github.com/user-attachments/assets/2ea64a34-4e57-4dec-a29e-32ffcd77919a)

After this change, customer has to set `OTEL_SEMCONV_STABILITY_OPT_IN`
to `database/dup`, in order to see the otel (new and old) attributes.
otherwise SDK will emit only classic attributes which would be
compatible with appinsights sdk also.

## Type of change
- [] Bug fix (non-breaking change which fixes an issue)

# Pull Request Template

## Description

Please include a summary of the change and which issue is fixed. Include
samples if adding new API, and include relevant motivation and context.
List any dependencies that are required for this change.

## Type of change

Please delete options that are not relevant.

- [] Bug fix (non-breaking change which fixes an issue)
- [] New feature (non-breaking change which adds functionality)
- [] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [] This change requires a documentation update

## Closing issues

To automatically close an issue: closes #IssueNumber

Co-authored-by: Kiran Kumar Kolli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Enables automation to merge PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants