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

Instrumentation.AWSLambda: Rename to remove .Contrib & minor cleanups. #593

Merged

Conversation

Oberon00
Copy link
Member

@Oberon00 Oberon00 commented Aug 18, 2022

Required before the release #590.

Changes

  • BREAKING: Rename package (& namespaces) to remove .Contrib.
  • BREAKING: Move public class AWSLambdaWrapper out of implementation subnamespace
  • BREAKING (technically): Make AWSLambdaWrapper a static class (found through [Instrumentation.AwsLambda] Enable public API check. #607).
  • Add version to ActivitySource

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

CC @srprash @rypdal

@utpilla utpilla added the comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda label Aug 19, 2022
@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Merging #593 (c9eab2b) into main (426f1a6) will increase coverage by 0.30%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #593      +/-   ##
==========================================
+ Coverage   76.40%   76.70%   +0.30%     
==========================================
  Files         170      170              
  Lines        5141     5161      +20     
==========================================
+ Hits         3928     3959      +31     
+ Misses       1213     1202      -11     
Impacted Files Coverage Δ
...ation.AWSLambda/AWSLambdaInstrumentationOptions.cs 100.00% <ø> (ø)
...Lambda/Implementation/AWSLambdaResourceDetector.cs 100.00% <ø> (ø)
...ntation.AWSLambda/Implementation/AWSLambdaUtils.cs 78.26% <ø> (ø)
...etry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs 94.82% <100.00%> (ø)
...ation.AWSLambda/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
...elemetry.Instrumentation.Process/ProcessMetrics.cs 100.00% <100.00%> (+100.00%) ⬆️
...entation.Process/MeterProviderBuilderExtensions.cs 100.00% <0.00%> (+100.00%) ⬆️

@Oberon00 Oberon00 marked this pull request as ready for review August 24, 2022 15:06
@Oberon00 Oberon00 requested a review from a team August 24, 2022 15:06
Copy link
Contributor

@Kielek Kielek left a comment

Choose a reason for hiding this comment

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

Any chance to make same changes for other AWS packages?

@Oberon00 Oberon00 changed the title Instrumentation.AWSLambda: Rename to remove .Contrib. Instrumentation.AWSLambda: Rename to remove .Contrib & minor cleanups. Aug 24, 2022
@Oberon00
Copy link
Member Author

Oberon00 commented Aug 24, 2022

@Kielek I'm only the owner of the AWS Lambda package, I think @srprash is responsible for AWS, and @srprash and @lupengamzn for X-Ray:

src/OpenTelemetry.Contrib.Extensions.AWSXRay/:
- srprash
- lupengamzn
src/OpenTelemetry.Contrib.Instrumentation.AWS/:
- srprash

@utpilla
Copy link
Contributor

utpilla commented Aug 24, 2022

@Oberon00 Please update the component_owners.yml file as well.

@Oberon00
Copy link
Member Author

Oberon00 commented Aug 26, 2022

@utpilla Done in 46464c2, I also noticed a few other references. I left the label name in the issue template as-is. I can change that as well if you like, but I think a maintainer should rename the label in the GitHub repository settings before (see #593 (comment)).

@Oberon00
Copy link
Member Author

Oberon00 commented Aug 26, 2022

Build failed due to unrelated problem which might be a race condition:

[xUnit.net 00:00:02.74]     OpenTelemetry.Instrumentation.Wcf.Tests.TelemetryClientMessageInspectorTests.OutgoingRequestInstrumentationTest(instrument: True, filter: False, suppressDownstreamInstrumentation: False, includeVersion: False, enrich: False, enrichmentException: False, emptyOrNullAction: False) [FAIL]
  Failed OpenTelemetry.Instrumentation.Wcf.Tests.TelemetryClientMessageInspectorTests.OutgoingRequestInstrumentationTest(instrument: True, filter: False, suppressDownstreamInstrumentation: False, includeVersion: False, enrich: False, enrichmentException: False, emptyOrNullAction: False) [1 ms]
  Error Message:
   System.ObjectDisposedException : Cannot access a disposed object.
Object name: 'System.Net.HttpListener'.
  Stack Trace:
     at System.Net.HttpListener.CheckDisposed()
   at System.Net.HttpListener.Stop()
   at OpenTelemetry.Instrumentation.Wcf.Tests.TelemetryClientMessageInspectorTests..ctor() in D:\a\opentelemetry-dotnet-contrib\opentelemetry-dotnet-contrib\test\OpenTelemetry.Instrumentation.Wcf.Tests\TelemetryClientMessageInspectorTests.cs:line 56
   at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean wrapExceptions)

@utpilla utpilla merged commit 1e8a13e into open-telemetry:main Aug 29, 2022
@Oberon00 Oberon00 deleted the feature/aws-lambda-rename branch September 6, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants