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

Bump OTel to 1.4.0 #1038

Merged
merged 19 commits into from
Feb 27, 2023
Merged

Bump OTel to 1.4.0 #1038

merged 19 commits into from
Feb 27, 2023

Conversation

Kielek
Copy link
Contributor

@Kielek Kielek commented Feb 24, 2023

Changes

  • Bump OTel 1.4.0 together with non-Core packages.
  • Exporter.Instana was pinned to 1.3.2. For now it is referencing net4.6.1
  • Adjust tests.
  • Tests executed only against netcoreapp3.1 are executed also on net7.0 and net6.0 (projects referencing OTel 1.4.0).

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • [ ] Design discussion issue #
  • [ ] Changes in public API reviewed

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #1038 (3d677e5) into main (1c7fd9a) will decrease coverage by 0.04%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1038      +/-   ##
==========================================
- Coverage   69.44%   69.40%   -0.04%     
==========================================
  Files         202      202              
  Lines        7668     7668              
==========================================
- Hits         5325     5322       -3     
- Misses       2343     2346       +3     
Impacted Files Coverage Δ
...ourceDetectors.Azure/AppServiceResourceDetector.cs 87.50% <ø> (ø)
...ckExchangeRedis/TracerProviderBuilderExtensions.cs 88.46% <0.00%> (-11.54%) ⬇️

@Kielek Kielek added dependencies Pull requests that update a dependency file .NET labels Feb 24, 2023
@Kielek Kielek changed the title [WIP] Bump OTel to 1.4.0 Bump OTel to 1.4.0 Feb 27, 2023
@Kielek Kielek marked this pull request as ready for review February 27, 2023 06:25
@Kielek Kielek requested a review from a team February 27, 2023 06:25
@cijothomas
Copy link
Member

Drop netcoreapp3.1 tests for projects based on OpenTelemetryCoreLatestVersion (to avoid compilation warnings)

^ We need to either revert this part, OR work-around the compiler warning, OR Get explicit approval from the component owners before merge.

@@ -17,7 +17,7 @@ using OpenTelemetry.Trace;
public void ConfigureServices(IServiceCollection services)
{
services.AddControllers();
services.AddOpenTelemetryTracing((builder) => builder
services.AddOpenTelemetry().WithTracing(builder => builder
Copy link
Contributor

@utpilla utpilla Feb 27, 2023

Choose a reason for hiding this comment

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

I don't think if we should simply update the README with the new methods.

It could be the case that a lot of the users of AWS instrumentation still use older version of OTel and the extensions hosting package (considering this project still references OTel SDK 1.0.1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will revert this file. Other redmes looks good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Kielek
Copy link
Contributor Author

Kielek commented Feb 27, 2023

Drop netcoreapp3.1 tests for projects based on OpenTelemetryCoreLatestVersion (to avoid compilation warnings)

^ We need to either revert this part, OR work-around the compiler warning, OR Get explicit approval from the component owners before merge.

I will check the workaround. As I remember there is some switch for it. Give me around one hour

@Kielek
Copy link
Contributor Author

Kielek commented Feb 27, 2023

Drop netcoreapp3.1 tests for projects based on OpenTelemetryCoreLatestVersion (to avoid compilation warnings)

^ We need to either revert this part, OR work-around the compiler warning, OR Get explicit approval from the component owners before merge.

Fixed in 2ada9f6.
CI is still running.

Tests are failing for EventCournters for netcoreapp3.1, but I think it is expected per: open-telemetry/opentelemetry-dotnet#3767 (reply in thread) (test relay on metrics)

Removing it from pipeilene: 3d677e5

@hananiel, @mic-max could you please check if you are fine with this change?

@Kielek Kielek requested review from mic-max and hananiel February 27, 2023 19:27
@utpilla utpilla merged commit 3ca2b34 into open-telemetry:main Feb 27, 2023
Copy link
Contributor

@mic-max mic-max left a comment

Choose a reason for hiding this comment

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

LGTM wrt EventCounters 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.