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.

Sign up for GitHub

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

Jump to bottom

Add note on experimental semantic convention implementation, prefix semantics headers with experimental tag #970

Merged
merged 11 commits into from
Sep 13, 2021
Merged

Add note on experimental semantic convention implementation, prefix semantics headers with experimental tag #970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3db914e
add note on experimental semantic conv
lalitb Sep 3, 2021
7c37e80
nit
lalitb Sep 3, 2021
d76aa2a
Merge branch 'main' into experimental-sem-con
lalitb Sep 3, 2021
6445528
Document experimental feature in versioning doc
lalitb Sep 7, 2021
650b83f
Document experimental feature in versioning doc
lalitb Sep 7, 2021
bc85239
Merge branch 'main' into experimental-sem-con
lalitb Sep 9, 2021
f2fe591
rename experimental headers to experimental-*
lalitb Sep 9, 2021
26bff3f
review comments
lalitb Sep 9, 2021
653f90a
Merge branch 'experimental-sem-con' of github.com:lalitb/opentelemetr…
lalitb Sep 9, 2021
3138215
missing changes
lalitb Sep 10, 2021
2aa8755
fix bazel builg
lalitb Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Versioning.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ Refer to the [ABI Policy](./docs/abi-policy.md) for more details. To summarise:
allowed to break existing stable interfaces. Feature flags will be removed
once we have a stable implementation for the signal.

* As an exception, small experimental features in otherwise stable signals/components
mayn't necessarily be released under feature flag. These would be flagged as experimental
by adding a `NOTE` in it's header file - either at the beginning of file, or as the comment for
Copy link
Contributor

Choose a reason for hiding this comment

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

non-blocking comment -

Would it make sense to call the header file experimental_* to further clarify?

I think you could do this and still provide backwards compatibility later by forward-chain including the new non-experimental folder when it stabilizes. Just a thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this does makes sense. Will incorporate this.

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.

the experimental API methods. Also, if the complete header is experimental, it would be prefixed
as `experimental_`. As an example, the semantic conventions for
trace signal is experimental at the time of the writing and is within `experimental_semantic_conventions.h`

* Code under the "*::detail" namespace implements internal details,
and NOT part of public interface. Also, any API not documented in the [public
documentation](https://opentelemetry-cpp.readthedocs.io/en/latest/) is NOT part of public interface.
Expand Down
6 changes: 6 additions & 0 deletions ...pentelemetry/trace/semantic_conventions.h → ...trace/experimental_semantic_conventions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

// NOTE:
// This implementation is based on the experimental specs for trace semantic convention as defined
// here:
// https://github.com/open-telemetry/opentelemetry-specification/tree/v1.0.0/specification/trace/semantic_conventions
// and MAY will change in future.

#pragma once

#include "opentelemetry/common/string_util.h"
Expand Down
4 changes: 2 additions & 2 deletions examples/grpc/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ cc_library(
cc_binary(
name = "client_grpc",
srcs = [
"client.cpp",
"client.cc",
],
defines = ["BAZEL_BUILD"],
deps = [
Expand All @@ -47,7 +47,7 @@ cc_binary(
cc_binary(
name = "server_grpc",
srcs = [
"server.cpp",
"server.cc",
],
defines = ["BAZEL_BUILD"],
deps = [
Expand Down
2 changes: 1 addition & 1 deletion examples/grpc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ else()
endif()

foreach(_target client server)
add_executable(${_target} "${_target}.cpp")
add_executable(${_target} "${_target}.cc")
target_link_libraries(
${_target} example_grpc_proto protobuf::libprotobuf gRPC::grpc++
opentelemetry_trace opentelemetry_exporter_ostream_span)
Expand Down
24 changes: 12 additions & 12 deletions examples/grpc/client.cpp → examples/grpc/client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
// modern compilers are unaffected.
#include <grpcpp/grpcpp.h>
#ifdef BAZEL_BUILD
#include "examples/grpc/protos/messages.grpc.pb.h"
# include "examples/grpc/protos/messages.grpc.pb.h"
#else
#include "messages.grpc.pb.h"
# include "messages.grpc.pb.h"
#endif

#include "opentelemetry/trace/semantic_conventions.h"
#include "tracer_common.h"
#include <iostream>
#include <memory>
#include <string>
#include "opentelemetry/trace/experimental_semantic_conventions.h"
#include "tracer_common.h"

using grpc::Channel;
using grpc::ClientContext;
Expand All @@ -23,7 +23,6 @@ using grpc_example::Greeter;
using grpc_example::GreetRequest;
using grpc_example::GreetResponse;


namespace
{

Expand All @@ -45,13 +44,14 @@ class GreeterClient
options.kind = opentelemetry::trace::SpanKind::kClient;

std::string span_name = "GreeterClient/Greet";
auto span = get_tracer("grpc")->StartSpan(span_name,
{{OTEL_CPP_GET_ATTR(AttrRpcSystem), "grpc"},
{OTEL_CPP_GET_ATTR(AttrRpcService), "grpc-example.GreetService"},
{OTEL_CPP_GET_ATTR(AttrRpcMethod), "Greet"},
{OTEL_CPP_GET_ATTR(AttrNetPeerIp), ip},
{OTEL_CPP_GET_ATTR(AttrNetPeerPort), port}},
options);
auto span = get_tracer("grpc")->StartSpan(
span_name,
{{OTEL_CPP_GET_ATTR(AttrRpcSystem), "grpc"},
{OTEL_CPP_GET_ATTR(AttrRpcService), "grpc-example.GreetService"},
{OTEL_CPP_GET_ATTR(AttrRpcMethod), "Greet"},
{OTEL_CPP_GET_ATTR(AttrNetPeerIp), ip},
{OTEL_CPP_GET_ATTR(AttrNetPeerPort), port}},
options);

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

Expand Down
27 changes: 14 additions & 13 deletions examples/grpc/server.cpp → examples/grpc/server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#ifdef BAZEL_BUILD
#include "examples/grpc/protos/messages.grpc.pb.h"
# include "examples/grpc/protos/messages.grpc.pb.h"
#else
#include "messages.grpc.pb.h"
# include "messages.grpc.pb.h"
#endif

#include "opentelemetry/trace/context.h"
#include "opentelemetry/trace/semantic_conventions.h"
#include "opentelemetry/trace/experimental_semantic_conventions.h"
#include "opentelemetry/trace/span_context_kv_iterable_view.h"
#include "tracer_common.h"

Expand All @@ -16,10 +16,10 @@

#include <chrono>
#include <fstream>
#include <map>
#include <sstream>
#include <string>
#include <thread>
#include <map>

using grpc::Server;
using grpc::ServerBuilder;
Expand All @@ -31,7 +31,7 @@ using grpc_example::Greeter;
using grpc_example::GreetRequest;
using grpc_example::GreetResponse;

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

Expand All @@ -44,7 +44,8 @@ class GreeterServer final : public Greeter::Service
const GreetRequest *request,
GreetResponse *response) override
{
for( auto elem: context->client_metadata()) {
for (auto elem : context->client_metadata())
{
std::cout << "ELEM: " << elem.first << " " << elem.second << "\n";
}

Expand All @@ -61,13 +62,13 @@ class GreeterServer final : public Greeter::Service
options.parent = opentelemetry::trace::GetSpan(new_context)->GetContext();

std::string span_name = "GreeterService/Greet";
auto span = get_tracer("grpc")
->StartSpan(span_name,
{{OTEL_CPP_GET_ATTR(AttrRpcSystem), "grpc"},
{OTEL_CPP_GET_ATTR(AttrRpcService), "GreeterService"},
{OTEL_CPP_GET_ATTR(AttrRpcMethod), "Greet"},
{OTEL_CPP_GET_ATTR(AttrRpcGrpcStatusCode), 0}},
options);
auto span =
get_tracer("grpc")->StartSpan(span_name,
{{OTEL_CPP_GET_ATTR(AttrRpcSystem), "grpc"},
{OTEL_CPP_GET_ATTR(AttrRpcService), "GreeterService"},
{OTEL_CPP_GET_ATTR(AttrRpcMethod), "Greet"},
{OTEL_CPP_GET_ATTR(AttrRpcGrpcStatusCode), 0}},
options);
auto scope = get_tracer("grpc")->WithActiveSpan(span);

// Fetch and parse whatever HTTP headers we can from the gRPC request.
Expand Down
2 changes: 1 addition & 1 deletion examples/http/client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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

namespace
Expand Down
2 changes: 1 addition & 1 deletion examples/http/server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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

#include <iostream>
Expand Down
2 changes: 1 addition & 1 deletion exporters/jaeger/src/recordable.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/exporters/jaeger/recordable.h"
#include "opentelemetry/sdk/resource/semantic_conventions.h"
#include "opentelemetry/sdk/resource/experimental_semantic_conventions.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace exporter
Expand Down
2 changes: 1 addition & 1 deletion exporters/zipkin/src/recordable.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/exporters/zipkin/recordable.h"
#include "opentelemetry/sdk/resource/semantic_conventions.h"
#include "opentelemetry/sdk/resource/experimental_semantic_conventions.h"

#include <map>
#include <string>
Expand Down
9 changes: 9 additions & 0 deletions ...metry/sdk/resource/semantic_conventions.h → ...ource/experimental_semantic_conventions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

// NOTE:
// This implementation is based on the experimental specs for resource semantic convention as
// defined here:
// https://github.com/open-telemetry/opentelemetry-specification/tree/v1.0.0/specification/resource/semantic_conventions
// and MAY will change in future.

#pragma once

#include <type_traits>
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/resource/resource.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

#include "opentelemetry/sdk/resource/resource.h"
#include "opentelemetry/nostd/span.h"
#include "opentelemetry/sdk/resource/experimental_semantic_conventions.h"
#include "opentelemetry/sdk/resource/resource_detector.h"
#include "opentelemetry/sdk/resource/semantic_conventions.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/resource/resource_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include "opentelemetry/common/key_value_iterable_view.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/sdk/resource/experimental_semantic_conventions.h"
#include "opentelemetry/sdk/resource/resource_detector.h"
#include "opentelemetry/sdk/resource/semantic_conventions.h"

#include <cstdlib>
#include <string>
Expand Down