From a50e7cf8a3ffd955f0cb104f87e93a6ee379ca12 Mon Sep 17 00:00:00 2001 From: Tomasz Rojek Date: Thu, 13 May 2021 02:02:39 +0200 Subject: [PATCH] Bump otel-cpp dependency to 0.6.0 for Apache (#22) * Add codeowners Fixes #9 * Fix team names inside codeowners * Use CODEOWNERS from otel-cpp-contrib * Update opentelemetry-cpp dependency to revision 5278e8c * Fix tests to match how attributes are presented (it was broken by #632 commit 179a7f40) * Remove commented code --- instrumentation/httpd/WORKSPACE | 6 +-- instrumentation/httpd/src/otel/mod_otel.cpp | 45 ++++++++++--------- .../httpd/src/otel/opentelemetry.cpp | 11 ++--- .../httpd/tests/01-create-root-span.sh | 22 +++------ instrumentation/httpd/tests/tools.sh | 8 ++++ 5 files changed, 48 insertions(+), 44 deletions(-) diff --git a/instrumentation/httpd/WORKSPACE b/instrumentation/httpd/WORKSPACE index 873ba99a8..56dcf663d 100644 --- a/instrumentation/httpd/WORKSPACE +++ b/instrumentation/httpd/WORKSPACE @@ -3,10 +3,10 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") # Load OpentTelemetry-CPP dependency http_archive( name = "io_opentelemetry_cpp", - sha256 = "2621cb0efd9bae78a1f06866cf8e96417446a6a70568ed6f804a6cb91d916db1", - strip_prefix = "opentelemetry-cpp-bd68a22ff5f2343de68f9d56a68bc53ecd69d567", + sha256 = "59e16eab4f534907144882fe70a1ca4514cf720f7b8b6e2a2d999a1b1a9265c8", + strip_prefix = "opentelemetry-cpp-b4584adeaae259df89b33af884c641e70a60a7cf", urls = [ - "https://github.com/open-telemetry/opentelemetry-cpp/archive/bd68a22ff5f2343de68f9d56a68bc53ecd69d567.tar.gz" + "https://github.com/open-telemetry/opentelemetry-cpp/archive/b4584adeaae259df89b33af884c641e70a60a7cf.tar.gz" ], ) diff --git a/instrumentation/httpd/src/otel/mod_otel.cpp b/instrumentation/httpd/src/otel/mod_otel.cpp index 5556ff035..2485f336b 100644 --- a/instrumentation/httpd/src/otel/mod_otel.cpp +++ b/instrumentation/httpd/src/otel/mod_otel.cpp @@ -37,24 +37,27 @@ using namespace httpd_otel; const char kOpenTelemetryKeyNote[] = "OTEL"; const char kOpenTelemetryKeyOutboundNote[] = "OTEL_PROXY"; -static nostd::string_view HttpdGetter(const apr_table_t &hdrs, nostd::string_view trace_type) +class HttpdCarrier : public opentelemetry::context::propagation::TextMapCarrier { - auto fnd = apr_table_get(&hdrs, std::string(trace_type).c_str()); - return fnd ? fnd : ""; -} - -static void HttpdSetter(apr_table_t &hdrs, - nostd::string_view trace_type, - nostd::string_view trace_description) -{ - apr_table_set(&hdrs, std::string(trace_type).c_str(), - std::string(trace_description).c_str()); -} +public: + apr_table_t& hdrs; + HttpdCarrier(apr_table_t& headers):hdrs(headers){} + virtual nostd::string_view Get(nostd::string_view key) const noexcept override + { + auto fnd = apr_table_get(&hdrs, std::string(key).c_str()); + return fnd ? fnd : ""; + } + virtual void Set(nostd::string_view key, nostd::string_view value) noexcept override + { + apr_table_set(&hdrs, std::string(key).c_str(), + std::string(value).c_str()); + } +}; // propagators -opentelemetry::trace::propagation::HttpTraceContext PropagatorTraceContext; -opentelemetry::trace::propagation::B3Propagator PropagatorB3SingleHeader; -opentelemetry::trace::propagation::B3PropagatorMultiHeader PropagatorB3MultiHeader; +opentelemetry::trace::propagation::HttpTraceContext PropagatorTraceContext; +opentelemetry::trace::propagation::B3Propagator PropagatorB3SingleHeader; +opentelemetry::trace::propagation::B3PropagatorMultiHeader PropagatorB3MultiHeader; // from: // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-server-semantic-conventions @@ -123,16 +126,17 @@ static int opentel_handler(request_rec *r, int /* lookup_uri */ ) if (!config.ignore_inbound && config.propagation != OtelPropagation::NONE) { + HttpdCarrier car(*req->headers_in); opentelemetry::v0::context::Context ctx_new, ctx_cur = opentelemetry::context::RuntimeContext::GetCurrent(); switch (config.propagation) { default: - ctx_new = PropagatorTraceContext.Extract(HttpdGetter, *req->headers_in, ctx_cur); + ctx_new = PropagatorTraceContext.Extract(car, ctx_cur); break; case OtelPropagation::B3_SINGLE_HEADER: case OtelPropagation::B3_MULTI_HEADER: - ctx_new = PropagatorB3SingleHeader.Extract(HttpdGetter, *req->headers_in, ctx_cur); + ctx_new = PropagatorB3SingleHeader.Extract(car, ctx_cur); } req_data->token = opentelemetry::context::RuntimeContext::Attach(ctx_new); } @@ -221,16 +225,17 @@ static int proxy_fixup_handler(request_rec *r) // mod_proxy simply copies request headers from client therefore inject is into headers_in // instead of headers_out + HttpdCarrier car(*req->headers_in); switch (config.propagation) { case OtelPropagation::TRACE_CONTEXT: - PropagatorTraceContext.Inject(HttpdSetter, *req->headers_in, opentelemetry::context::RuntimeContext::GetCurrent()); + PropagatorTraceContext.Inject(car, opentelemetry::context::RuntimeContext::GetCurrent()); break; case OtelPropagation::B3_SINGLE_HEADER: - PropagatorB3SingleHeader.Inject(HttpdSetter, *req->headers_in, opentelemetry::context::RuntimeContext::GetCurrent()); + PropagatorB3SingleHeader.Inject(car, opentelemetry::context::RuntimeContext::GetCurrent()); break; case OtelPropagation::B3_MULTI_HEADER: - PropagatorB3MultiHeader.Inject(HttpdSetter, *req->headers_in, opentelemetry::context::RuntimeContext::GetCurrent()); + PropagatorB3MultiHeader.Inject(car, opentelemetry::context::RuntimeContext::GetCurrent()); break; default: // suppress warning break; diff --git a/instrumentation/httpd/src/otel/opentelemetry.cpp b/instrumentation/httpd/src/otel/opentelemetry.cpp index 2aae4a1e8..ec9ca635c 100644 --- a/instrumentation/httpd/src/otel/opentelemetry.cpp +++ b/instrumentation/httpd/src/otel/opentelemetry.cpp @@ -73,16 +73,17 @@ void initTracer() break; } - std::shared_ptr processor; + std::unique_ptr processor; if (config.batch_opts.max_queue_size) { - processor = std::make_shared( - std::move(exporter), config.batch_opts); + processor = std::unique_ptr( + new sdktrace::BatchSpanProcessor(std::move(exporter), config.batch_opts)); } else { - processor = std::make_shared(std::move(exporter)); + processor = std::unique_ptr( + new sdktrace::SimpleSpanProcessor(std::move(exporter))); } // add custom-configured resources @@ -93,7 +94,7 @@ void initTracer() } auto provider = nostd::shared_ptr( - new sdktrace::TracerProvider(processor, + new sdktrace::TracerProvider(std::move(processor), opentelemetry::sdk::resource::Resource::Create(resAttrs)) ); diff --git a/instrumentation/httpd/tests/01-create-root-span.sh b/instrumentation/httpd/tests/01-create-root-span.sh index c67cb1524..2d4d0c8a3 100755 --- a/instrumentation/httpd/tests/01-create-root-span.sh +++ b/instrumentation/httpd/tests/01-create-root-span.sh @@ -28,22 +28,12 @@ check_results() { [ "`getSpanField span_id`" != "`getSpanField parent_span_id`" ] || fail "Bad span: span.id same as parent span.id" echo Checking span attributes - declare -A SPAN_ATTRS - # transforms "http.method: GET, http.flavor: http, ..." into SPAN_ATTRS[http.method] = GET - IFS=',' read -ra my_array <<< "`getSpanField attributes`" - for i in "${my_array[@]}"; do - TMP_KEY=${i%%:*} - TMP_KEY=${TMP_KEY# } - TMP_VAL=${i##*:} - TMP_VAL=${TMP_VAL:1} - SPAN_ATTRS[${TMP_KEY}]="${TMP_VAL}" - done - - [[ "${SPAN_ATTRS[http.method]}" == "GET" ]] || fail "Bad span attribiute http.method ${SPAN_ATTRS[http.method]}" - [[ "${SPAN_ATTRS[http.target]}" == "/" ]] || fail "Bad span attribiute http.target ${SPAN_ATTRS[http.target]}" - [[ "${SPAN_ATTRS[http.status_code]}" == "200" ]] || fail "Bad span attribiute http.status_code ${SPAN_ATTRS[http.status_code]}" - [[ "${SPAN_ATTRS[http.flavor]}" == "1.1" ]] || fail "Bad span attribiute http.flavor ${SPAN_ATTRS[http.flavor]}" - [[ "${SPAN_ATTRS[http.scheme]}" == "http" ]] || fail "Bad span attribiute http.scheme ${SPAN_ATTRS[http.scheme]}" + + [[ "`getSpanAttr 'http.method'`" == "GET" ]] || fail "Bad span attribiute http.method ${SPAN_ATTRS[http.method]}" + [[ "`getSpanAttr 'http.target'`" == "/" ]] || fail "Bad span attribiute http.target ${SPAN_ATTRS[http.target]}" + [[ "`getSpanAttr 'http.status_code'`" == "200" ]] || fail "Bad span attribiute http.status_code ${SPAN_ATTRS[http.status_code]}" + [[ "`getSpanAttr 'http.flavor'`" == "1.1" ]] || fail "Bad span attribiute http.flavor ${SPAN_ATTRS[http.flavor]}" + [[ "`getSpanAttr 'http.scheme'`" == "http" ]] || fail "Bad span attribiute http.scheme ${SPAN_ATTRS[http.scheme]}" } run $@ diff --git a/instrumentation/httpd/tests/tools.sh b/instrumentation/httpd/tests/tools.sh index 77e95f4a0..39a715e9e 100755 --- a/instrumentation/httpd/tests/tools.sh +++ b/instrumentation/httpd/tests/tools.sh @@ -45,6 +45,14 @@ count() { echo OK Found $TOTAL occurence\(s\) of "$1" } +# returns one span attribute +getSpanAttr() { + LINE=`grep "attributes :" ${OUTPUT_SPANS} -A 20 | grep " $1" ` + VALUE=`echo $LINE | cut -d ':' -f 2-` + VALUE="${VALUE## }" + echo $VALUE +} + # returns one span field getSpanField() { WHICHONE=${2-1}