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] Enable public API check. #607

Merged

Conversation

Oberon00
Copy link
Member

Enable public API check for Instrumentation.AwsLambda, as I think that's a very useful tool.

No CHANGELOG entry.

@Oberon00 Oberon00 force-pushed the feature/awslambda-apicheck branch from 7a2eadf to 1ef074b Compare August 24, 2022 16:06
@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #607 (febe06c) into main (e7bc1b0) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #607   +/-   ##
=======================================
  Coverage   76.70%   76.70%           
=======================================
  Files         170      170           
  Lines        5161     5161           
=======================================
  Hits         3959     3959           
  Misses       1202     1202           
Impacted Files Coverage Δ
...etry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs 94.82% <100.00%> (ø)

@Oberon00 Oberon00 marked this pull request as ready for review August 24, 2022 16:36
@Oberon00 Oberon00 requested a review from a team August 24, 2022 16:36
@utpilla utpilla added the comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda label Aug 24, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2022

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added Stale and removed Stale labels Sep 1, 2022
@Oberon00 Oberon00 force-pushed the feature/awslambda-apicheck branch from a7aa258 to febe06c Compare September 5, 2022 09:40
Copy link
Contributor

@rypdal rypdal left a comment

Choose a reason for hiding this comment

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

Changes are looking good and can be merged.

@@ -5,6 +5,7 @@
<Description>AWS Lambda tracing wrapper for OpenTelemetry .NET</Description>
<PackageTags>$(PackageTags);AWS Lambda</PackageTags>
<MinVerTagPrefix>Instrumentation.AWSLambda-</MinVerTagPrefix>
<EnablePublicApi>true</EnablePublicApi>
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

That is disabled by default? Weird, how should I know 😅
I will try to enable it.
Maybe it would be better to enable it by default and explicitly disable it in projects where it causes problems?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried it locally, the tool only suggested adding some ConfigureAwait(false) in some places. I will submit a PR for that eventually, but I don't want to hold up the release longer, so I will do that after #590

@utpilla utpilla merged commit bf01da1 into open-telemetry:main Sep 6, 2022
@Oberon00 Oberon00 deleted the feature/awslambda-apicheck branch September 8, 2022 07:33
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.

3 participants