Review and enhance tracing support for currency service (C++)

[cartersocha]

The following is a list of requirements that we need to evaluate before declaring v1 for trace telemetry. These requirements are across the entire application; Not all services will meet all requirements. Determine the relevant features for this service.

• Automatic Instrumentation is being used where appropriate.
• Library instrumentation is being used if automatic instrumentation is unavailable.
• Services extend automatic instrumentation.
  • New attributes/events attached to existing spans.
  • New spans are being created from existing spans.
• Automatic and Manual Context Propagation are being demonstrated.
• Telemetry is not being sampled upfront.
• Telemetry is not being filtered upfront.
• Baggage is being set and read appropriately (i.e., baggage must be explicitly set as attributes on spans)
• Update the centralized tracing doc

Referencing: #42

Service: https://github.com/open-telemetry/opentelemetry-demo-webstore/blob/main/src/currencyservice/README.md

C++ dependent on this item: #36

[lalitb]

@DebajitDas - Do you have plans to take this further?

[marcalff]

Please note that opentelemetry-cpp contains recent changes that have some impact on the C++ demo here.

SDK builders

[TRACE SDK] Add trace sdk builders (#1393) open-telemetry/opentelemetry-cpp#1471

This change provides a new set of API to use to configure the SDK.

Code similar to:

auto exporter = std::unique_ptr<opentelemetry::sdk::trace::SpanExporter>(
      new opentelemetry::exporter::otlp::OtlpGrpcExporter(opts));

creates issues with shared libraries, and is to be avoided.

Instead, please use:

auto exporter = opentelemetry::exporter::otlp::OtlpGrpcExporterFactory::Create(opts);

and likewise for all SDK configuration.

Semantic conventions

[SEMANTIC CONVENTIONS] Upgrade to version 1.12.0 open-telemetry/opentelemetry-cpp#873

Code like:

{OTEL_GET_TRACE_ATTR(AttrRpcSystem), "grpc"},

is better replaced by:

{SemanticConventions::RPC_SYSTEM, "grpc"},

New release to come

These two changes are unreleased, with opentelemetry-cpp 1.5.0 coming soon.

We appreciate if the opentelemetry-demo code can show case the best practices,
because the code will serve as an example for many people to build from.

Regards,
-- Marc

[DebajitDas]

@DebajitDas - Do you have plans to take this further?

Yes @lalitb, I will be looking into this. Please assign it to me. Thanks

[lalitb]

@cartersocha - Can you assign this to @DebajitDas please? Thanks.

[lalitb]

Adding my comments on the feature list in the description

• Automatic Instrumentation is being used where appropriate. : Nothing to be done as automatic instrumentation is not available for C++
• Library instrumentation is being used if automatic instrumentation is unavailable. : Nothing to be done as automatic instrumentation is not available for C++
• Services extend automatic instrumentation. : Nothing to be done as automatic instrumentation is not available for C++
• New attributes/events attached to existing spans. : Demonstrate Span::AddEvent and Span::SetAttribute by adding new(random) events and attributes ( if not done already).
• New spans are being created from existing spans. : This is already implemented, as we create a new span from the span-context coming from the client using trace-propagation.
• Automatic and Manual Context Propagation are being demonstrated.: Nothing to be done as Manual Context propagation is already implemented. Automatic Context Propagation is not available.
• Telemetry is not being sampled upfront.: Nothing to be done as sampling is not done in code.
• Telemetry is not being filtered upfront.: Nothing to to be done as filtering is not done in code.
• Baggage is being set and read appropriately (i.e., baggage must be explicitly set as attributes on spans): Baggage needs to be read from the request header, and set as Span attributes (Example to read the baggage - https://github.com/open-telemetry/opentelemetry-cpp/blob/78551fbb535e2381fcdc6a1585b32f4999b43794/api/test/baggage/baggage_test.cc#L83)
• Update the centralized tracing doc - Need to link relevant sections from otel-cpp documentation (https://opentelemetry-cpp.readthedocs.io/en/latest/)

Also, there are couple of comments from @marcalff to be handled -

• Use SDK builders for SDK configuration - Refer to the updated tracer_common.h for grpc here - https://github.com/open-telemetry/opentelemetry-cpp/blob/main/examples/grpc/tracer_common.h
• Semantic Conventions - The usage has been updated in the latest otel-cpp, refer to updated server.cc for grpc here - https://github.com/open-telemetry/opentelemetry-cpp/blob/main/examples/grpc/server.cc

[cartersocha]

updated @lalitb. For baggage we're waiting for the new frontend to be merged then that will start the baggage process.

Seems like the only other missing piece besides baggage & documentation updates is: " Demonstrate Span::AddEvent and Span::SetAttribute by adding new(random) events and attributes ( if not done already).??

[lalitb]

Seems like the only other missing piece besides baggage & documentation updates is: " Demonstrate Span::AddEvent and Span::SetAttribute by adding new(random) events and attributes ( if not done already).??

Yes, that's correct, apart from the two changes I mentioned separately.