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

Remove dependency from OpenTelemetry.Instrumentation.StackExchangeRedis #3104

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

sebastienros
Copy link
Member

@sebastienros sebastienros commented Mar 22, 2024

Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Mar 22, 2024
@sebastienros sebastienros changed the title Remove dependency from OpenTelemetry.Instrumentation.SqlClient Remove dependency from OpenTelemetry.Instrumentation.StackExchangeRedis Mar 22, 2024
@sebastienros
Copy link
Member Author

Since the /Shared files from open-telemetry and open-telemetry-contrib are different each library has its own in their vendoring folder.

@sebastienros
Copy link
Member Author

sebastienros commented Mar 22, 2024

Tests are broken by the change at await notifier.ActivityReceived.WaitAsync(TimeSpan.FromSeconds(10));

Update:
Solved, this is because the OTel library are computing the ActivitySourceName from the assembly name. I am now checking other vendor-copy PRs for the same problem.

git fetch --tags
git checkout tags/Instrumentation.SqlClient-1.7.0-beta.1
git checkout tags/Instrumentation.StackExchangeRedis-1.0.0-rc9.13
Copy link
Member

Choose a reason for hiding this comment

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

What's your thoughts on putting these versions in? As a way to document which version we are currently on? How do we plan on keeping it up to date?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to show which versions are used right now. When there is a new release this has to be updated. Otherwise there is no way to know which is currently used and if work needs to be done. Also the way the otel repos is done each lib is release individually which doesn't help.

@sebastienros sebastienros merged commit ac2b068 into main Mar 26, 2024
8 checks passed
@sebastienros sebastienros deleted the sebros/stack branch March 26, 2024 21:02
@sebastienros
Copy link
Member Author

/backport to release/8.0-preview5

Copy link
Contributor

Started backporting to release/8.0-preview5: https://github.com/dotnet/aspire/actions/runs/8442856355

Copy link
Contributor

@sebastienros backporting to release/8.0-preview5 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Remove dependency from OpenTelemetry.Instrumentation.StackExchangeRedis
Using index info to reconstruct a base tree...
M	Directory.Packages.props
M	src/Components/Aspire.Microsoft.Data.SqlClient/Aspire.Microsoft.Data.SqlClient.csproj
M	src/Components/Aspire.Microsoft.EntityFrameworkCore.SqlServer/Aspire.Microsoft.EntityFrameworkCore.SqlServer.csproj
M	src/Vendoring/.editorconfig
M	src/Vendoring/README.md
Falling back to patching base and 3-way merge...
Auto-merging src/Vendoring/README.md
CONFLICT (content): Merge conflict in src/Vendoring/README.md
Auto-merging src/Components/Aspire.Microsoft.EntityFrameworkCore.SqlServer/Aspire.Microsoft.EntityFrameworkCore.SqlServer.csproj
Auto-merging src/Components/Aspire.Microsoft.Data.SqlClient/Aspire.Microsoft.Data.SqlClient.csproj
Auto-merging Directory.Packages.props
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Remove dependency from OpenTelemetry.Instrumentation.StackExchangeRedis
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

Copy link
Contributor

@sebastienros an error occurred while backporting to release/8.0-preview5, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants