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

Adding semantic-convention attributes for trace #868

Merged
merged 12 commits into from
Jul 30, 2021
190 changes: 190 additions & 0 deletions api/include/opentelemetry/trace/semantic_conventions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#pragma once

#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/version.h"

#define OTEL_TRACE_GENERATE_SEMCONV_METHOD(attribute_name, attribute_value) \
static const nostd::string_view Get##attribute_name() noexcept \
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this should be expressible as constexpr - to return the value of it at compile time, without incurring the overhead of a static method call. I'm not sure if the getters give significant benefit, as in - better structure or something? From Google C++ style document: Global strings: if you require a global or static string constant, consider using a simple character array, or a char pointer to the first element of a string literal. String literals have static storage duration already and are usually sufficient.

Copy link
Member Author

@lalitb lalitb Jun 21, 2021

Choose a reason for hiding this comment

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

How can we do it in an all header way without getters? C++11 won't allow initializing static std::string/nostd::string within class definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

You just don't use getters. Use constants with constexpr definition?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or if you use getters, return straight the literal constant type without any class:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Or if you use getters, return straight the literal constant type without any class:

Thanks for your suggestions. I have modified the code to create the constrxpr function and return literal. For another suggestion on using the constrxpr variable, somehow, I don't see these getting optimized in different translation units by gcc compiler ( small pingy code to demonstrate that: https://www.onlinegdb.com/IjRRjLjZo ). But anyway preferred the constexpr function as we don't need to depend on linker/compiler optimization here.

Copy link
Member Author

Choose a reason for hiding this comment

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

To summarise the available options:
option1: constexpr static class methods: The advantage of using these is that only those methods are generated (in .text segment) at compile time which are used across all the translation units. The rest of the unused methods are ignored.
option2: constexpr static global functions in the namespace: This will create a copy of the used functions in each translation unit while ignoring the rest of the unused functions.
option3: constexpt static variables in the namespace: This will create a local copy of all the variables in each of the translation units including the header, and it seems that the gcc compiler doesn't optimize it.

The right solution comes from the C++17 inline variables, but option1 seems to be the correct approach for C++11 which is now implemented in this PR. We can discuss if there are any issues with the approach.

{ \
static const nostd::string_view attribute_name = attribute_value; \
return attribute_name; \
}

OPENTELEMETRY_BEGIN_NAMESPACE
namespace trace
{

/**
* Stores the Constants for semantic kAttribute names outlined by the OpenTelemetry specifications.
* <see
* href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/README.md"/>.
*/
class SemanticConventions final
{
public:
// The set of constants matches the specification as of this commit.
// https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/trace/semantic_conventions
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md

// Genetal network connection attributes
lalitb marked this conversation as resolved.
Show resolved Hide resolved
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetTransport, "net.transport")
Copy link
Contributor

Choose a reason for hiding this comment

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

Expand Attribute in name AttributeNetTransport in the macro of OTEL_TRACE_GENERATE_SEMCONV_METHOD instead of duplicating it for every macro call?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Will do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is done now.

OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetPeerIp, "net.peer.ip")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we generate these definitions from the YAML definition of semantic conventions such as https://github.com/open-telemetry/opentelemetry-specification/blob/main/semantic_conventions/trace/http.yaml?

Copy link
Contributor

Choose a reason for hiding this comment

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

...and could we generate them, and make them constexpr string constants? These would be encouraged by Google C++ coding style, and provide the most optimal compile-time optimization. Global strings: if you require a global or static string constant, consider using a simple character array, or a char pointer to the first element of a string literal. String literals have static storage duration already and are usually sufficient. .. constexpr string literal or string array should work, with all of these constants having unique prefix. Rather than grouping them in class - we group them in namespace. There is also a compile time flag that allows string pooling, we use that to avoid bloating in case if the same header gets included in different compilation units. For Visual Studio compiler that is /GF (Eliminate Duplicate Strings). And for gcc / clang I think it's on by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could we generate these definitions from the YAML definition of semantic conventions such as

I did think of writing a script to parse the yaml file and generate C++ code containing these consts from that and add it as part of CI. But this data is not going to change on daily basis, and instead, it would be fine to modify our code a couple of times before GA to be consistent.
For otel-python and otel-go, they do have a script to generate code for these constants, along with other semantic data ( eg creating enums for allowable values for attributes, defining constraints of the list of attributes to be used etc) which we don't need to do. The current approach in this PR is as done by otel-dotnet.

Copy link
Member Author

@lalitb lalitb Jun 23, 2021

Choose a reason for hiding this comment

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

@ThomsonTan - Have created issue for automatic generation of these definitions : #873
Once we finalize the approach to use, first ( and probably initial few ) definitions can be committed manually while someone works on a script to generate code as per the finalized approach. Hope that works.

OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetPeerPort, "net.peer.port")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetPeerName, "net.peer.name")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetHostIp, "net.host.ip")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetHostPort, "net.host.port")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeNetHostName, "net.host.name")

// General identity attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeEnduserId, "enduser.id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeEnduserRole, "enduser.role")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeEnduserScope, "enduser.scope")

// General remote service attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributePeerService, "peer.service")

// General thread attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeThreadId, "thread.id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeThreadName, "thread.name")

// Source Code Attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeCodeFunction, "code.function")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeCodeNamespace, "code.namespace")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeCodeFilepath, "code.filepath")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeCodeLineno, "code.lineno")

// Http Span Common Attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpMethod, "http.method")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpUrl, "http.url")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpTarget, "http.target")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpHost, "http.host")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpScheme, "http.scheme")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpStatusCode, "http.status_code")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpFlavor, "http.flavor")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpUserAgent, "http.user_agent")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpRequestContentLength,
"http.request_content_length")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpRequestContentLengthUncompressed,
"http.request_content_length_uncompressed")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpResponseContentLength,
"http.response_content_length")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpResponseContentLengthUncompressed,
"http.response_content_length_uncompressed")

// HTTP Span Client Attributes
// One of the following combinations:
// - http.url
// - http.scheme, http.host, http.target
// - http.scheme, net.peer.name, net.peer.port, http.target
// - http.scheme, net.peer.ip, net.peer.port, http.target

// HTTP Span Server Attributes
// One of the following combinations:
// -http.scheme, http.host, http.target
// -http.scheme, http.server_name, net.host.port, http.target
// -http.scheme, net.host.name, net.host.port, http.target
// -http.url
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpServerName, "http.server_name")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpRoute, "http.route")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeHttpClientIp, "http.client_ip")

// DB: Connection-level attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbSystem, "db.system") // other_sql, mssql, mysql...
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbConnectionString, "db.connection_string")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbUser, "db.user")
// DB: Connection-level attributes for specific technologies
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbMssqlInstanceName, "db.mssql.instance_name")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbJdbcDriverClassname, "db.jdbc.driver_classname")
// DB: Call-level attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbName, "db.name")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbStatement, "db.statement")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbOperation, "db.operation")
// DB: Call-level attributes for specific technologies
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbHbaseNamespace, "db.hbase.namespace")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbRedisDatabaseIndex, "db.redis.database_index")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbMongodbCollection, "db.mongodb.collection")

// // DB: Call-level attributes for Cassandra for clarity
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraKeyspace, "db.cassandra.keyspace")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraPageSize, "db.cassandra.page_size")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraConsistencyLevel,
"db.cassandra.consistency_level")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraTable, "db.cassandra.table")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraIdempotence, "db.cassandra.idempotence")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraSpeculativeExecutionCount,
"db.cassandra.speculative_execution_count")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraCoordinatorId,
"db.cassandra.coordinator.id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeDbCassandraCoordinatorDC,
"db.cassandra.coordinator.dc")

// Common RPC attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcSystem, "rpc.system")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcService, "rpc.service")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcMethod, "rpc.method")
// gRPC attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcGrpcStatusCode, "rpc.grpc.status_code")
// JSON-RPC attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcJsonrpcVersion, "rpc.jsonrpc.version")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcJsonrpcRequestId, "rpc.jsonrpc.request_id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcJsonrpcErrorCode, "rpc.jsonrpc.error_code")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeRpcJsonrpcErrorMessage, "rpc.jsonrpc.error_message")

// faas: General Attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasTrigger, "faas.trigger")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasExecution, "faas.execution")
// faas: incoming invocations
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasColdStart, "faas.coldstart")
// faas: outgoing invocations
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasInvokedName, "faas.invoked_name")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasInvokedProvider, "faas.invoked_provider")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasInvokedRegion, "faas.invoked_region")
// faas: datastore trigger
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasDocumentCollection, "faas.document.collection")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasDocumentOperation, "faas.document.operation")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasDocumentTime, "faas.document.time")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasDocumentName, "faas.document.name")
// faas: timer trigger
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasTime, "faas.time")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeFaasCron, "faas.cron")

// messaging attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingSystem, "messaging.system")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingDestination, "messaging.destination")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingDestinationKind,
"messaging.destination_kind")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingTempDestination,
"messaging.temp_destination")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingProtocol, "messaging.protocol")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingProtocolVersion,
"messaging.protocol_version")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingUrl, "messaging.url")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingMessageId, "messaging.message_id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingConversationId, "messaging.conversation_id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingPayloadSize,
"messaging.message_payload_size_bytes")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingPayloadCompressedSize,
"messaging.message_payload_compressed_size_bytes")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingOperation, "messaging.operation")
// messaging attributes specific to messaging systems
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingRabbitMQRoutingKey,
"messaging.rabbitmq.routing_key")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingKafkaMessageKey,
"messaging.kafka.message_key")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingKafkaConsumerGroup,
"messaging.kafka.consumer_group")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingKafkaClientId, "messaging.kafka.client_id")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingKafkaPartition, "messaging.kafka.partition")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeMessagingKafkaTombstone, "messaging.kafka.tombstone")

// Exceptions attributes
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeExceptionType, "exception.type")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeExceptionMessage, "exception.message")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeExceptionStacktrace, "exception.stacktrace")
OTEL_TRACE_GENERATE_SEMCONV_METHOD(AttributeExceptionEscapted, "exception.escaped")
};
} // namespace trace
OPENTELEMETRY_END_NAMESPACE
16 changes: 9 additions & 7 deletions examples/grpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <grpcpp/grpcpp.h>
#include "messages.grpc.pb.h"

#include "opentelemetry/trace/semantic_conventions.h"
#include "tracer_common.h"
#include <iostream>
#include <memory>
Expand All @@ -22,6 +23,7 @@ using grpc_example::GreetResponse;
namespace
{

using namespace opentelemetry::trace;
class GreeterClient
{
public:
Expand All @@ -40,11 +42,11 @@ class GreeterClient

std::string span_name = "GreeterClient/Greet";
auto span = get_tracer("grpc")->StartSpan(span_name,
{{"rpc.system", "grpc"},
{"rpc.service", "grpc-example.GreetService"},
{"rpc.method", "Greet"},
{"net.peer.ip", ip},
{"net.peer.port", port}},
{{SemanticConventions::GetAttributeRpcSystem(), "grpc"},
{SemanticConventions::GetAttributeRpcService(), "grpc-example.GreetService"},
{SemanticConventions::GetAttributeRpcMethod(), "Greet"},
{SemanticConventions::GetAttributeNetPeerIp(), ip},
{SemanticConventions::GetAttributeNetPeerPort(), port}},
options);

auto scope = get_tracer("grpc-client")->WithActiveSpan(span);
Expand All @@ -60,7 +62,7 @@ class GreeterClient
if (status.ok())
{
span->SetStatus(opentelemetry::trace::StatusCode::kOk);
span->SetAttribute("rpc.grpc.status_code", status.error_code());
span->SetAttribute(SemanticConventions::GetAttributeRpcGrpcStatusCode(), status.error_code());
// Make sure to end your spans!
span->End();
return response.response();
Expand All @@ -69,7 +71,7 @@ class GreeterClient
{
std::cout << status.error_code() << ": " << status.error_message() << std::endl;
span->SetStatus(opentelemetry::trace::StatusCode::kError);
span->SetAttribute("rpc.grpc.status_code", status.error_code());
span->SetAttribute(SemanticConventions::GetAttributeRpcGrpcStatusCode(), status.error_code());
// Make sure to end your spans!
span->End();
return "RPC failed";
Expand Down
10 changes: 6 additions & 4 deletions examples/grpc/server.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "messages.grpc.pb.h"
#include "tracer_common.h"
#include "opentelemetry/trace/span_context_kv_iterable_view.h"
#include "opentelemetry/trace/semantic_conventions.h"

#include <grpcpp/grpcpp.h>
#include <grpcpp/server.h>
Expand All @@ -26,6 +27,7 @@ using grpc_example::GreetResponse;

using Span = opentelemetry::trace::Span;
using SpanContext = opentelemetry::trace::SpanContext;
using SemanticConventions = opentelemetry::trace::SemanticConventions;

namespace
{
Expand Down Expand Up @@ -55,10 +57,10 @@ class GreeterServer final : public Greeter::Service
std::string span_name = "GreeterService/Greet";
auto span = get_tracer("grpc")
->StartSpan(span_name,
{{"rpc.system", "grpc"},
{"rpc.service", "GreeterService"},
{"rpc.method", "Greet"},
{"rpc.grpc.status_code", 0}},
{{SemanticConventions::GetAttributeRpcSystem(), "grpc"},
{SemanticConventions::GetAttributeRpcService(), "GreeterService"},
{SemanticConventions::GetAttributeRpcMethod(), "Greet"},
{SemanticConventions::GetAttributeRpcGrpcStatusCode(), 0}},
options);
auto scope = get_tracer("grpc")->WithActiveSpan(span);

Expand Down
11 changes: 7 additions & 4 deletions examples/http/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@

#include "opentelemetry/ext/http/client/http_client_factory.h"
#include "opentelemetry/ext/http/common/url_parser.h"
#include "opentelemetry/trace/semantic_conventions.h"
#include "tracer_common.h"

namespace
{

using namespace opentelemetry::trace;

void sendRequest(const std::string &url)
{
auto http_client = opentelemetry::ext::http::client::HttpClientFactory::CreateSync();
Expand All @@ -20,9 +23,9 @@ void sendRequest(const std::string &url)
std::string span_name = url_parser.path_;
auto span = get_tracer("http-client")
->StartSpan(span_name,
{{"http.url", url_parser.url_},
{"http.scheme", url_parser.scheme_},
{"http.method", "GET"}},
{{SemanticConventions::GetAttributeHttpUrl(), url_parser.url_},
{SemanticConventions::GetAttributeHttpScheme(), url_parser.scheme_},
{SemanticConventions::GetAttributeHttpMethod(), "GET"}},
options);
auto scope = get_tracer("http-client")->WithActiveSpan(span);

Expand All @@ -38,7 +41,7 @@ void sendRequest(const std::string &url)
{
// set span attributes
auto status_code = result.GetResponse().GetStatusCode();
span->SetAttribute("http.status_code", status_code);
span->SetAttribute(SemanticConventions::GetAttributeHttpStatusCode(), status_code);
result.GetResponse().ForEachHeader([&span](opentelemetry::nostd::string_view header_name,
opentelemetry::nostd::string_view header_value) {
span->SetAttribute("http.header." + std::string(header_name.data()), header_value);
Expand Down
25 changes: 14 additions & 11 deletions examples/http/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
// SPDX-License-Identifier: Apache-2.0

#include "server.h"
#include "opentelemetry/trace/semantic_conventions.h"
#include "tracer_common.h"

#include <iostream>
#include <thread>

namespace
{

using namespace opentelemetry::trace;
;
uint16_t server_port = 8800;
constexpr const char *server_name = "localhost";

Expand All @@ -31,17 +35,16 @@ class RequestHandler : public HTTP_SERVER_NS::HttpRequestCallback
options.parent = opentelemetry::trace::propagation::GetSpan(new_context)->GetContext();

// start span with parent context extracted from http header
auto span =
get_tracer("http-server")
->StartSpan(
span_name,
{{"http.server_name", server_name},
{"net.host.port", server_port},
{"http.method", request.method},
{"http.scheme", "http"},
{"http.request_content_length", static_cast<uint64_t>(request.content.length())},
{"http.client_ip", request.client}},
options);
auto span = get_tracer("http-server")
->StartSpan(span_name,
{{SemanticConventions::GetAttributeHttpServerName(), server_name},
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks rather bulky, in my opinion.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. Do we have a better way of handling this?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are several options:

  • make methods constexpr and return a string literal constant, inited as const char * or char array
  • or just define all string constants in a dedicated namespace, that enables using for those who want to write it in a shorter form. Then for gcc / clang this scenario is already optimized, and for msvc we need to enable string pooling to combine all strings into one region of memory. These constants all have global static duration, there is no need to "get" them, they'd be efficiently compile-time optimized. Using string_view for that is an overkill.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I did initially wanted to use the second option but wasn't aware of the compiler optimization of this static global across multiple translation units. Let me try that.

Copy link
Member Author

Choose a reason for hiding this comment

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

As mentioned in prev comment, have created the constexpr function now.

{SemanticConventions::GetAttributeNetHostPort(), server_port},
{SemanticConventions::GetAttributeHttpMethod(), request.method},
{SemanticConventions::GetAttributeHttpScheme(), "http"},
{SemanticConventions::GetAttributeHttpRequestContentLength(),
static_cast<uint64_t>(request.content.length())},
{SemanticConventions::GetAttributeHttpClientIp(), request.client}},
options);

auto scope = get_tracer("http_server")->WithActiveSpan(span);

Expand Down