[EXPORTER] Allow to share gRPC clients between OTLP exporters #3041

owent · 2024-08-29T07:00:09Z

Changes

Add APIs to allow to share gRPC clients between OTLP exporters.
gRPC depends abseil-cpp, so we always enable abseil-cpp when have WITH_OTLP_GRPC
Keep grpc::Channel in OtlpGrpcClient so we can share it between multiple exporters.

We can use OtlpGrpcClientFactory::Create() to create an OtlpGrpcClient. This client can then be passed to OtlpGrpcExporterFactory::Create(), OtlpGrpcLogRecordExporterFactory::Create(), and OtlpGrpcMetricExporterFactory::Create(). By doing this, all gRPC exporters created by these methods will share the same OtlpGrpcClient and utilize the same grpc::CompletionQueue and grpc::Channel.

When the exporters call Shutdown, the OtlpGrpcClient will only call ForceFlush and will not Shutdown unless all the exporters using this client object are also shutdown.We can use OtlpGrpcClientFactory::Create() to create OtlpGrpcClient now, and then pass it to OtlpGrpcExporterFactory::Create() , OtlpGrpcLogRecordExporterFactory::Create() and OtlpGrpcMetricExporterFactory::Create() , so all gRPC exporters created by these methods will share the same OtlpGrpcClient and use the same grpc::CompletionQueue and grpc::Channel.

When the exporters call Shutdown, the OtlpGrpcClient just call ForceFlush but do not Shutdown unless all the exporters use this client object are shutdown.

Usage example:

OtlpGrpcClientOptions grpc_opts;
grpc_opts.endpoint = "localhost:12345";
// ... other initializations

OtlpGrpcExporterOptions trace_opts = grpc_opts;
OtlpGrpcLogRecordExporterOptions logs_opts = grpc_opts;

nostd::shared_ptr<OtlpGrpcClient> shared_client = OtlpGrpcClientFactory::Create(OtlpGrpcLogRecordExporterOptions());
auto trace_exporter = OtlpGrpcExporterFactory::Create(trace_opts, shared_client);
auto logs_exporter = OtlpGrpcLogRecordExporterFactory::Create(logs_opts, shared_client);

// ...

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

CHANGELOG.md updated for non-trivial changes
Unit tests have been added
Changes in public API reviewed

codecov · 2024-08-29T07:13:39Z

Codecov Report

Attention: Patch coverage is 64.93506% with 27 lines in your changes missing coverage. Please review.

Project coverage is 87.83%. Comparing base (a388e87) to head (62756bd).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...metry/nostd/internal/absl/types/internal/variant.h	59.71%	27 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3041   +/-   ##
=======================================
  Coverage   87.83%   87.83%           
=======================================
  Files         195      195           
  Lines        6146     6146           
=======================================
  Hits         5398     5398           
  Misses        748      748

Files with missing lines	Coverage Δ
...lemetry/nostd/internal/absl/base/internal/invoke.h	`100.00% <ø> (ø)`
...try/nostd/internal/absl/types/bad_variant_access.h	`100.00% <ø> (ø)`
.../opentelemetry/nostd/internal/absl/types/variant.h	`100.00% <100.00%> (ø)`
...pentelemetry/nostd/internal/absl/utility/utility.h	`100.00% <100.00%> (ø)`
api/include/opentelemetry/nostd/variant.h	`66.67% <ø> (ø)`
...metry/nostd/internal/absl/types/internal/variant.h	`74.57% <59.71%> (ø)`

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

lalitb · 2024-08-29T09:14:05Z

Thanks @owent. It would be helpful to have API details in the PR description. And possibly an example here to understand the usage - as a separate PR if not possible here :)

…H_OTLP_GRPC`

owent · 2024-08-30T02:48:01Z

Thanks @owent. It would be helpful to have API details in the PR description. And possibly an example here to understand the usage - as a separate PR if not possible here :)

Thanks, the details are added. I can add examples in another PR later.

…grpc_client

netlify · 2024-09-12T15:14:03Z

✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name	Link
🔨 Latest commit	`62756bd`
🔍 Latest deploy log	https://app.netlify.com/sites/opentelemetry-cpp-api-docs/deploys/673bb6aa5bcffa00089d108f

…grpc_client

owent · 2024-09-24T07:17:37Z

Is there any change to include this PR into next release?

marcalff · 2024-09-24T21:31:37Z

Is there any change to include this PR into next release?

Hi @owent.

Going though the code review, and I would like to include it in the next release as well.

In the PR description, there are two descriptions for "In async mode, ..." but none for the sync mode.

Could you revise the text for clarity, I assume you wanted two sections, one for sync and one for async.

…grpc_client

marcalff · 2024-11-04T20:23:52Z

Sorry for requesting change. This is for - #3041 (comment)

If we are agreeing to not being ABI compliant when using OTLP/gRPC exporter, it would be better to update our ABI document to reflect that as part of this PR. No concern as such with the implementation.

I think this is now resolved by the latest changes in internal abseil, by @owent.

Using the OTLP GRPC exporter requires external abseil, which no longer forces the API to use abseil, and therefore does not change the ABI for nostd::variant.

Let's discuss technical details in the team meeting.

…exporters_with_existed_grpc_client

…grpc_client

owent · 2024-11-14T03:10:20Z

Sorry for requesting change. This is for - #3041 (comment)
If we are agreeing to not being ABI compliant when using OTLP/gRPC exporter, it would be better to update our ABI document to reflect that as part of this PR. No concern as such with the implementation.

I think this is now resolved by the latest changes in internal abseil, by @owent.

Using the OTLP GRPC exporter requires external abseil, which no longer forces the API to use abseil, and therefore does not change the ABI for nostd::variant.

Let's discuss technical details in the team meeting.

Yes, the issue is resolved by making the internal Abseil-CPP namespace non-inline.
I also checked all include paths and there should be no issues now.

…grpc_client

marcalff · 2024-11-15T07:54:20Z

Ping @lalitb . Issue with ABSEIL and ABI is resolved, please take another look.

lalitb

Removing the change requested, as both the concerns - using external abseil for nostd, and use of nostd constructs in sdk - have been resolved. Thanks for fixing this.

fixed/

…grpc_client

[EXPORTER] Allow to share gRPC clients between OTLP exporters (open-telemetry#3041)

malkia · 2024-11-19T17:34:14Z

There is a typo -> shitdown should be shutdown :) - made some of my coworkes chuckle today :) hahahaha

Allow to share gRPC clients between OTLP exporters.

213e3cf

owent requested a review from a team August 29, 2024 07:00

Add changelog, fix compiling problems without async

5c5f9b7

owent added 2 commits August 29, 2024 19:41

gRPC depends abseil-cp, so we always enable abseil-cpp when have `WIT…

37e5081

…H_OTLP_GRPC`

Fix linking error

c9cb2df

owent changed the title ~~Allow to share gRPC clients between OTLP exporters.~~ [WIP] Allow to share gRPC clients between OTLP exporters. Aug 30, 2024

owent added 2 commits August 30, 2024 11:55

Share grpc::Channel

0d409d4

Fix compiling error

cd80d19

owent changed the title ~~[WIP] Allow to share gRPC clients between OTLP exporters.~~ Allow to share gRPC clients between OTLP exporters. Aug 30, 2024

owent added 5 commits August 30, 2024 20:36

Make OtlpGrpcClientReferenceGuard always available

ad722f4

Try to cancel running requests when shutdown timeout

f4521f3

Modify running_calls do not depends the thread for gRPC callback.

38860d3

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

b5daf3f

…grpc_client

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

8e9d300

…grpc_client

marcalff assigned marcalff, ThomsonTan and esigo Sep 9, 2024

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

51e0964

…grpc_client

owent and others added 2 commits September 18, 2024 15:17

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

40ca747

…grpc_client

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

b2aa191

…grpc_client

marcalff requested a review from a team as a code owner September 23, 2024 06:52

marcalff changed the title ~~Allow to share gRPC clients between OTLP exporters.~~ [EXPORTER] Allow to share gRPC clients between OTLP exporters. Sep 24, 2024

marcalff mentioned this pull request Sep 24, 2024

[RELEASE] Release opentelemetry-cpp 1.17.0 #3069

Closed

Allow to share gRPC Client in sync mode

3547800

owent added 3 commits October 31, 2024 00:44

Do not inline otabsl to avoid namespace conflicts

5142036

Fix markdown lint

d7266aa

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

603e2fa

…grpc_client

owent mentioned this pull request Nov 4, 2024

Clarifications needed around WITH_ABSEIL #3116

Closed

owent and others added 5 commits November 5, 2024 14:08

Merge remote-tracking branch 'github/main' into allow_construct_grpc_…

adcf88e

…exporters_with_existed_grpc_client

Add internal logs when shutdown gRPC Client

b71c655

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

1145003

…grpc_client

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

4d4d3a5

…grpc_client

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

fe30625

…grpc_client

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

7b2f29e

…grpc_client

marcalff mentioned this pull request Nov 15, 2024

[RELEASE] Prepare release opentelemetry-cpp 1.18.0 #3145

Closed

lalitb reviewed Nov 18, 2024

View reviewed changes

marcalff added the ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve) label Nov 18, 2024

Merge branch 'main' into allow_construct_grpc_exporters_with_existed_…

62756bd

…grpc_client

marcalff changed the title ~~[EXPORTER] Allow to share gRPC clients between OTLP exporters.~~ [EXPORTER] Allow to share gRPC clients between OTLP exporters Nov 18, 2024

marcalff merged commit a5db7e5 into open-telemetry:main Nov 18, 2024
56 checks passed

malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Nov 18, 2024

Merge pull request #90 from open-telemetry/main

dbe50d8

[EXPORTER] Allow to share gRPC clients between OTLP exporters (open-telemetry#3041)

ThomsonTan mentioned this pull request Nov 19, 2024

[BUILD] Fix error message #3152

Merged

3 tasks

marcalff mentioned this pull request Nov 23, 2024

[INSTALL] Fix cmake/opentelemetry-cpp-config.cmake.in #3165

Merged

3 tasks

BrewTestBot mentioned this pull request Nov 26, 2024

opentelemetry-cpp 1.18.0 Homebrew/homebrew-core#199015

Merged

This was referenced Nov 28, 2024

Cmake Amazon Linux 2023 Build fails for WITH_OTLP_GRPC=ON #2586

Closed

Build with external abseil not working, ambiguous references #3063

Closed

owent mentioned this pull request Dec 2, 2024

Compile errors occur when using gcc 14 #2591

Open

owent deleted the allow_construct_grpc_exporters_with_existed_grpc_client branch December 12, 2024 06:12

owent mentioned this pull request Jan 29, 2025

[DOC] Add document and example for sharing gRPC Client #3260

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[EXPORTER] Allow to share gRPC clients between OTLP exporters #3041

[EXPORTER] Allow to share gRPC clients between OTLP exporters #3041

Uh oh!

owent commented Aug 29, 2024 •

edited

Loading

Uh oh!

codecov bot commented Aug 29, 2024 •

edited

Loading

Uh oh!

lalitb commented Aug 29, 2024

Uh oh!

owent commented Aug 30, 2024 •

edited

Loading

Uh oh!

netlify bot commented Sep 12, 2024 •

edited

Loading

Uh oh!

owent commented Sep 24, 2024

Uh oh!

marcalff commented Sep 24, 2024

Uh oh!

marcalff commented Nov 4, 2024

Uh oh!

owent commented Nov 14, 2024

Uh oh!

marcalff commented Nov 15, 2024

Uh oh!

lalitb left a comment

Uh oh!

Uh oh!

malkia commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[EXPORTER] Allow to share gRPC clients between OTLP exporters #3041

[EXPORTER] Allow to share gRPC clients between OTLP exporters #3041

Uh oh!

Conversation

owent commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

codecov bot commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lalitb commented Aug 29, 2024

Uh oh!

owent commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.

Uh oh!

owent commented Sep 24, 2024

Uh oh!

marcalff commented Sep 24, 2024

Uh oh!

marcalff commented Nov 4, 2024

Uh oh!

owent commented Nov 14, 2024

Uh oh!

marcalff commented Nov 15, 2024

Uh oh!

lalitb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

malkia commented Nov 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

owent commented Aug 29, 2024 •

edited

Loading

codecov bot commented Aug 29, 2024 •

edited

Loading

owent commented Aug 30, 2024 •

edited

Loading

netlify bot commented Sep 12, 2024 •

edited

Loading