Skip to content

Remove BuiltInMeters()#2900

Merged
eerhardt merged 6 commits intomicrosoft:mainfrom
martincostello:Remove-AddBuiltInMeters
Mar 15, 2024
Merged

Remove BuiltInMeters()#2900
eerhardt merged 6 commits intomicrosoft:mainfrom
martincostello:Remove-AddBuiltInMeters

Conversation

@martincostello
Copy link
Copy Markdown
Contributor

@martincostello martincostello commented Mar 14, 2024

Remove some extra BuiltInMeters() code that snuck in after #1948.

Microsoft Reviewers: Open in CodeFlow

Remove some extra `BuiltInMeters()` code that snuck in after microsoft#1948.
@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 14, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 14, 2024
Apply review feedback.
Copy link
Copy Markdown
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

Thanks for taking them all!

//.AddGrpcClientInstrumentation()
.AddHttpClientInstrumentation()
// Add instrumentation for the AWS .NET SDK.
.AddAWSInstrumentation();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't use AWS here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry - copy-paste fail.

Comment on lines +68 to +69
// Uncomment the following line to enable gRPC instrumentation (requires the OpenTelemetry.Instrumentation.GrpcNetClient package)
//.AddGrpcClientInstrumentation()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the TestShop does in fact use Grpc, can we keep this one? That way we get the right tracing in the TestShop.

martincostello and others added 4 commits March 14, 2024 13:49
- Add gRPC back to the test shop.
- Remove incorrect AWS references.
Put it back so it builds...
Add another one back.
- Remove OpenTelemetry.Instrumentation.Process from Directory.Packages.props
- Sync a few more ServiceDefaults with the template
Copy link
Copy Markdown
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution!

@eerhardt eerhardt enabled auto-merge (squash) March 15, 2024 03:09
@eerhardt eerhardt merged commit a57c102 into microsoft:main Mar 15, 2024
@martincostello martincostello deleted the Remove-AddBuiltInMeters branch March 15, 2024 14:09
@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2024
@github-actions github-actions bot added area-integrations Issues pertaining to Aspire Integrations packages and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 9, 2025
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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants