Add OTLP Metric Exporter Factory and `OtlpGrpcClient` #1606

owent · 2022-09-09T15:55:07Z

Signed-off-by: owent [email protected]

Fixes #1603
Fixes #1605

Changes

Add OtlpGrpcMetricExporterFactory and OtlpHttpMetricExporterFactory.
Fix compatibility of OTLP log unit tests for MSVC.
Add OtlpGrpcClient

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 · 2022-09-09T16:04:49Z

Codecov Report

Merging #1606 (30e4586) into main (951768a) will decrease coverage by 0.07%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1606      +/-   ##
==========================================
- Coverage   85.26%   85.20%   -0.06%     
==========================================
  Files         156      156              
  Lines        4978     4978              
==========================================
- Hits         4244     4241       -3     
- Misses        734      737       +3

Impacted Files	Coverage Δ
ext/src/http/client/curl/http_client_curl.cc	`80.31% <0.00%> (-1.13%)`	⬇️

lalitb · 2022-09-09T18:09:23Z

@owent - you may want to fix the conflict from #1601 for this PR :)

owent · 2022-09-09T19:00:14Z

@owent - you may want to fix the conflict from #1601 for this PR :)

Done

esigo

LGTM
Thanks for the PR :)

lalitb · 2022-09-12T05:04:14Z

Did a quick validations on the binaries generated from this PR.

It seems that example_otlp_grpc_log still has dependency on both libopentelemetry_exporter_otlp_grpc.so and libopentelemetry_exporter_otlp_grpc_log.so. Should that be fixed?

$ ldd example_otlp_grpc_log
        linux-vdso.so.1 (0x00007fff86bf5000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f4b4ef1b000)
        libcommon_logs_foo_library.so => /tmp/owent/opentelemetry-cpp/build/examples/common/logs_foo_library/libcommon_logs_foo_library.so (0x00007f4b4eeb6000)
        libopentelemetry_exporter_otlp_grpc.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc.so (0x00007f4b4edff000)
        libopentelemetry_exporter_otlp_grpc_log.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc_log.so (0x00007f4b4ed48000)
        libopentelemetry_trace.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/trace/libopentelemetry_trace.so (0x00007f4b4ec4a000)
        libopentelemetry_logs.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/logs/libopentelemetry_logs.so (0x00007f4b4eb63000)
        libopentelemetry_exporter_otlp_grpc_client.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc_client.so (0x00007f4b4e5e2000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f4b4e400000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f4b4e3e5000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f4b4e1f3000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f4b4f1d1000)
        libopentelemetry_otlp_recordable.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_otlp_recordable.so (0x00007f4b4dec1000)
        libopentelemetry_common.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/common/libopentelemetry_common.so (0x00007f4b4deae000)
        libopentelemetry_resources.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/resource/libopentelemetry_resources.so (0x00007f4b4ddd1000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f4b4dc82000)
        libssl.so.1.1 => /lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007f4b4dbef000)
        libcrypto.so.1.1 => /lib/x86_64-linux-gnu/libcrypto.so.1.1 (0x00007f4b4d917000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f4b4d911000)

While example_otlp_grpc is rightly dependent on only libopentelemetry_exporter_otlp_grpc.so.

$ ldd ./example_otlp_grpc
        linux-vdso.so.1 (0x00007fff1a3e1000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f6543907000)
        libcommon_foo_library.so => /tmp/owent/opentelemetry-cpp/build/examples/common/foo_library/libcommon_foo_library.so (0x00007f65438ac000)
        libopentelemetry_exporter_otlp_grpc.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc.so (0x00007f65437f5000)
        libopentelemetry_trace.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/trace/libopentelemetry_trace.so (0x00007f65436f9000)
        libopentelemetry_exporter_otlp_grpc_client.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc_client.so (0x00007f6543176000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f6542f94000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f6542f79000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f6542d87000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f6543b93000)
        libopentelemetry_otlp_recordable.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_otlp_recordable.so (0x00007f6542a57000)
        libopentelemetry_common.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/common/libopentelemetry_common.so (0x00007f6542a42000)
        libopentelemetry_resources.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/resource/libopentelemetry_resources.so (0x00007f6542965000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f6542816000)
        libssl.so.1.1 => /lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007f6542783000)
        libcrypto.so.1.1 => /lib/x86_64-linux-gnu/libcrypto.so.1.1 (0x00007f65424ad000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f65424a5000)

lalitb · 2022-09-12T05:04:58Z

Fix compatibility of OTLP log unit tests for MSVC.

What is the compatibility issue here? Thanks.

owent · 2022-09-12T05:27:03Z

Did a quick validations on the binaries generated from this PR.

It seems that example_otlp_grpc_log still has dependency on both libopentelemetry_exporter_otlp_grpc.so and libopentelemetry_exporter_otlp_grpc_log.so. Should that be fixed?

$ ldd example_otlp_grpc_log
        linux-vdso.so.1 (0x00007fff86bf5000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f4b4ef1b000)
        libcommon_logs_foo_library.so => /tmp/owent/opentelemetry-cpp/build/examples/common/logs_foo_library/libcommon_logs_foo_library.so (0x00007f4b4eeb6000)
        libopentelemetry_exporter_otlp_grpc.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc.so (0x00007f4b4edff000)
        libopentelemetry_exporter_otlp_grpc_log.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc_log.so (0x00007f4b4ed48000)
        libopentelemetry_trace.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/trace/libopentelemetry_trace.so (0x00007f4b4ec4a000)
        libopentelemetry_logs.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/logs/libopentelemetry_logs.so (0x00007f4b4eb63000)
        libopentelemetry_exporter_otlp_grpc_client.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc_client.so (0x00007f4b4e5e2000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f4b4e400000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f4b4e3e5000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f4b4e1f3000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f4b4f1d1000)
        libopentelemetry_otlp_recordable.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_otlp_recordable.so (0x00007f4b4dec1000)
        libopentelemetry_common.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/common/libopentelemetry_common.so (0x00007f4b4deae000)
        libopentelemetry_resources.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/resource/libopentelemetry_resources.so (0x00007f4b4ddd1000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f4b4dc82000)
        libssl.so.1.1 => /lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007f4b4dbef000)
        libcrypto.so.1.1 => /lib/x86_64-linux-gnu/libcrypto.so.1.1 (0x00007f4b4d917000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f4b4d911000)

While example_otlp_grpc is rightly dependent on only libopentelemetry_exporter_otlp_grpc.so.

$ ldd ./example_otlp_grpc
        linux-vdso.so.1 (0x00007fff1a3e1000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f6543907000)
        libcommon_foo_library.so => /tmp/owent/opentelemetry-cpp/build/examples/common/foo_library/libcommon_foo_library.so (0x00007f65438ac000)
        libopentelemetry_exporter_otlp_grpc.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc.so (0x00007f65437f5000)
        libopentelemetry_trace.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/trace/libopentelemetry_trace.so (0x00007f65436f9000)
        libopentelemetry_exporter_otlp_grpc_client.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_exporter_otlp_grpc_client.so (0x00007f6543176000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f6542f94000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f6542f79000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f6542d87000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f6543b93000)
        libopentelemetry_otlp_recordable.so => /tmp/owent/opentelemetry-cpp/build/exporters/otlp/libopentelemetry_otlp_recordable.so (0x00007f6542a57000)
        libopentelemetry_common.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/common/libopentelemetry_common.so (0x00007f6542a42000)
        libopentelemetry_resources.so => /tmp/owent/opentelemetry-cpp/build/sdk/src/resource/libopentelemetry_resources.so (0x00007f6542965000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f6542816000)
        libssl.so.1.1 => /lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007f6542783000)
        libcrypto.so.1.1 => /lib/x86_64-linux-gnu/libcrypto.so.1.1 (0x00007f65424ad000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f65424a5000)

Yes, but libopentelemetry_exporter_otlp_grpc_log.so and libopentelemetry_exporter_otlp_grpc.so do not link grpc anymore.

owent · 2022-09-12T05:32:40Z

Fix compatibility of OTLP log unit tests for MSVC.

What is the compatibility issue here? Thanks.

When compiling with the latest MSVC (17.3) , the std::string array can not convert to opentelemetry::common::AttributeValue .
It seems to have some problem to convrert to span<string_view> implicitly.

lalitb · 2022-09-12T17:26:46Z

Yes, but libopentelemetry_exporter_otlp_grpc_log.so and libopentelemetry_exporter_otlp_grpc.so do not link grpc anymore.

Yes, I can see opentelemetry_exporter_otlp_grpc_client adding the dependency on grpc which seems to be fine. But was just thinking that the actual solution could have been to remove libopentelemetry_exporter_otlp_grpc.so dependency for example_otlp_grpc_log ?

lalitb · 2022-09-12T17:33:27Z

Yes, but libopentelemetry_exporter_otlp_grpc_log.so and libopentelemetry_exporter_otlp_grpc.so do not link grpc anymore.

>Yes, I can see opentelemetry_exporter_otlp_grpc_client adding the dependency on grpc which seems to be fine. But was just thinking that the actual solution could have been to remove libopentelemetry_exporter_otlp_grpc.so dependency for example_otlp_grpc_log ?

Please ignore this. I just realized it is possible for an instrumentation library to have a dependency on both log and trace exporters.

lalitb · 2022-09-12T18:33:28Z

Another nit comment, now that the libgrpc.a and libprotobuf.a are statically added in opentelemetry_exporter_otlp_grpc_client.so, we can remove the gRPC dependency from otlp exporter example -

opentelemetry-cpp/examples/otlp/CMakeLists.txt

Line 9 in 951768a

opentelemetry_trace opentelemetry_exporter_otlp_grpc gRPC::grpc++)

Not blocker for this PR as if required it can done later in #1598

lalitb

LGTM with nit comments. Thanks for the fix.

…ory`. + Fix compatibility of OTLP log unit tests for MSVC. Signed-off-by: owent <[email protected]>

Signed-off-by: owent <[email protected]>

owent · 2022-09-13T02:24:25Z

LGTM with nit comments. Thanks for the fix.

Thanks and gRPC::grpc++ is removed from OTLP examples now.

…#1606)

owent requested a review from a team September 9, 2022 15:55

owent changed the title ~~[WIP] Add OTLP Metric Exporter Factory and OtlpGrpcClient~~ Add OTLP Metric Exporter Factory and OtlpGrpcClient Sep 9, 2022

owent force-pushed the fix_1603_and_add_otlp_metric_factory branch from 83c88ae to 9580025 Compare September 9, 2022 18:39

esigo approved these changes Sep 10, 2022

View reviewed changes

lalitb mentioned this pull request Sep 12, 2022

Example for OTLP gRPC exporter for Metrics. #1598

Merged

3 tasks

lalitb approved these changes Sep 12, 2022

View reviewed changes

owent added 4 commits September 13, 2022 10:17

+ Add OtlpGrpcMetricExporterFactory and `OtlpHttpMetricExporterFact…

0fb4803

…ory`. + Fix compatibility of OTLP log unit tests for MSVC. Signed-off-by: owent <[email protected]>

Add OtlpGrpcClient

8685a9b

Signed-off-by: owent <[email protected]>

Fix format

c6bb677

Signed-off-by: owent <[email protected]>

Remove gRPC::gprc++ from example.

30e4586

Signed-off-by: owent <[email protected]>

owent force-pushed the fix_1603_and_add_otlp_metric_factory branch from 4581b85 to 30e4586 Compare September 13, 2022 02:23

lalitb merged commit 0b7d4ab into open-telemetry:main Sep 13, 2022

owent deleted the fix_1603_and_add_otlp_metric_factory branch October 4, 2022 06:47

yxue pushed a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022

Add OTLP Metric Exporter Factory and OtlpGrpcClient (open-telemetry…

097ec4e

…#1606)

yxue pushed a commit to yxue/opentelemetry-cpp that referenced this pull request Dec 5, 2022

Add OTLP Metric Exporter Factory and OtlpGrpcClient (open-telemetry…

45c8808

…#1606)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OTLP Metric Exporter Factory and `OtlpGrpcClient` #1606

Add OTLP Metric Exporter Factory and `OtlpGrpcClient` #1606

owent commented Sep 9, 2022 •

edited

Loading

codecov bot commented Sep 9, 2022 •

edited

Loading

lalitb commented Sep 9, 2022

owent commented Sep 9, 2022

esigo left a comment

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb commented Sep 12, 2022

owent commented Sep 12, 2022

owent commented Sep 12, 2022

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb left a comment

owent commented Sep 13, 2022

Add OTLP Metric Exporter Factory and OtlpGrpcClient #1606

Add OTLP Metric Exporter Factory and OtlpGrpcClient #1606

Conversation

owent commented Sep 9, 2022 • edited Loading

Changes

codecov bot commented Sep 9, 2022 • edited Loading

Codecov Report

lalitb commented Sep 9, 2022

owent commented Sep 9, 2022

esigo left a comment

Choose a reason for hiding this comment

lalitb commented Sep 12, 2022 • edited Loading

lalitb commented Sep 12, 2022

owent commented Sep 12, 2022

owent commented Sep 12, 2022

lalitb commented Sep 12, 2022 • edited Loading

lalitb commented Sep 12, 2022 • edited Loading

lalitb commented Sep 12, 2022 • edited Loading

lalitb left a comment

Choose a reason for hiding this comment

owent commented Sep 13, 2022

Add OTLP Metric Exporter Factory and `OtlpGrpcClient` #1606

Add OTLP Metric Exporter Factory and `OtlpGrpcClient` #1606

owent commented Sep 9, 2022 •

edited

Loading

codecov bot commented Sep 9, 2022 •

edited

Loading

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb commented Sep 12, 2022 •

edited

Loading

lalitb commented Sep 12, 2022 •

edited

Loading