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

[API] Add InstrumentationScope attributes in MeterProvider::GetMeter() #2224

Merged
merged 21 commits into from
Sep 30, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
32 changes: 32 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,38 @@ jobs:
run: |
(cd ./functional/otlp; ./run_test.sh)

cmake_clang_maintainer_abiv2_test:
name: CMake clang 14 (maintainer mode, abiv2)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: 'recursive'
- name: setup
env:
CC: /usr/bin/clang-14
CXX: /usr/bin/clang++-14
PROTOBUF_VERSION: 21.12
run: |
sudo -E ./ci/setup_cmake.sh
sudo -E ./ci/setup_ci_environment.sh
sudo -E ./ci/install_protobuf.sh
- name: run cmake clang (maintainer mode, abiv2)
env:
CC: /usr/bin/clang-14
CXX: /usr/bin/clang++-14
run: |
./ci/do_ci.sh cmake.maintainer.abiv2.test
- name: generate test cert
env:
CFSSL_VERSION: 1.6.3
run: |
sudo -E ./tools/setup-cfssl.sh
(cd ./functional/cert; ./generate_cert.sh)
- name: run func test
run: |
(cd ./functional/otlp; ./run_test.sh)

cmake_msvc_maintainer_test:
name: CMake msvc (maintainer mode)
runs-on: windows-latest
Expand Down
54 changes: 49 additions & 5 deletions api/include/opentelemetry/metrics/meter_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@

#pragma once

#include "opentelemetry/common/key_value_iterable.h"
#include "opentelemetry/common/key_value_iterable_view.h"
#include "opentelemetry/nostd/shared_ptr.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/nostd/type_traits.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand All @@ -20,15 +23,56 @@ class MeterProvider
{
public:
virtual ~MeterProvider() = default;

#if OPENTELEMETRY_ABI_VERSION_NO >= 2
/**
* Gets or creates a named Meter instance.
*
* Optionally a version can be passed to create a named and versioned Meter
* instance.
* @since ABI_VERSION 2
*
* @param[in] name Meter instrumentation scope
* @param[in] version Instrumentation scope version, optional
* @param[in] schema_url Instrumentation scope schema URL, optional
* @param[in] attributes Instrumentation scope attributes, optional
*/
virtual nostd::shared_ptr<Meter> GetMeter(
nostd::string_view name,
nostd::string_view version = "",
nostd::string_view schema_url = "",
const common::KeyValueIterable *attributes = nullptr) noexcept = 0;
#else
/**
* Gets or creates a named Meter instance.
*
* @since ABI_VERSION 1
*
* @param[in] name Meter instrumentation scope
* @param[in] version Instrumentation scope version, optional
* @param[in] schema_url Instrumentation scope schema URL, optional
*/
virtual nostd::shared_ptr<Meter> GetMeter(nostd::string_view library_name,
nostd::string_view library_version = "",
nostd::string_view schema_url = "") noexcept = 0;
virtual nostd::shared_ptr<Meter> GetMeter(nostd::string_view name,
nostd::string_view version = "",
nostd::string_view schema_url = "") noexcept = 0;
#endif

#if OPENTELEMETRY_ABI_VERSION_NO >= 2
nostd::shared_ptr<Meter> GetMeter(
nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url,
std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>> attributes)
{
nostd::span<const std::pair<nostd::string_view, common::AttributeValue>> attributes_span{
attributes.begin(), attributes.end()};

common::KeyValueIterableView<
nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>>
iterable_attributes{attributes_span};

return GetMeter(name, version, schema_url, &iterable_attributes);
}
marcalff marked this conversation as resolved.
Show resolved Hide resolved
#endif

#ifdef ENABLE_REMOVE_METER_PREVIEW
marcalff marked this conversation as resolved.
Show resolved Hide resolved
virtual void RemoveMeter(nostd::string_view library_name,
nostd::string_view library_version = "",
Expand Down
15 changes: 13 additions & 2 deletions api/include/opentelemetry/metrics/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,23 @@ class NoopMeterProvider final : public MeterProvider
public:
NoopMeterProvider() : meter_{nostd::shared_ptr<Meter>(new NoopMeter)} {}

nostd::shared_ptr<Meter> GetMeter(nostd::string_view /* library_name */,
nostd::string_view /* library_version */,
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
nostd::shared_ptr<Meter> GetMeter(
nostd::string_view /* name */,
nostd::string_view /* version */,
nostd::string_view /* schema_url */,
const common::KeyValueIterable * /* attributes */) noexcept override
{
return meter_;
}
#else
nostd::shared_ptr<Meter> GetMeter(nostd::string_view /* name */,
nostd::string_view /* version */,
nostd::string_view /* schema_url */) noexcept override
{
return meter_;
}
#endif

#ifdef ENABLE_REMOVE_METER_PREVIEW
void RemoveMeter(nostd::string_view /* name */,
Expand Down
25 changes: 25 additions & 0 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,31 @@ elif [[ "$1" == "cmake.maintainer.cpp11.async.test" ]]; then
make -k -j $(nproc)
make test
exit 0
elif [[ "$1" == "cmake.maintainer.abiv2.test" ]]; then
cd "${BUILD_DIR}"
rm -rf *
cmake ${CMAKE_OPTIONS[@]} \
-DWITH_OTLP_HTTP=ON \
-DWITH_OTLP_HTTP_SSL_PREVIEW=ON \
-DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=ON \
-DWITH_REMOVE_METER_PREVIEW=ON \
-DWITH_PROMETHEUS=ON \
-DWITH_EXAMPLES=ON \
-DWITH_EXAMPLES_HTTP=ON \
-DWITH_ZIPKIN=ON \
-DBUILD_W3CTRACECONTEXT_TEST=ON \
-DWITH_ELASTICSEARCH=ON \
-DWITH_METRICS_EXEMPLAR_PREVIEW=ON \
-DWITH_ASYNC_EXPORT_PREVIEW=OFF \
-DOTELCPP_MAINTAINER_MODE=ON \
-DWITH_NO_DEPRECATED_CODE=ON \
-DWITH_ABI_VERSION_1=OFF \
-DWITH_ABI_VERSION_2=ON \
${IWYU} \
"${SRC_DIR}"
eval "$MAKE_COMMAND"
make test
exit 0
elif [[ "$1" == "cmake.with_async_export.test" ]]; then
cd "${BUILD_DIR}"
rm -rf *
Expand Down
17 changes: 15 additions & 2 deletions sdk/include/opentelemetry/sdk/common/attribute_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ struct AttributeConverter
class AttributeMap : public std::unordered_map<std::string, OwnedAttributeValue>
{
public:
// Contruct empty attribute map
// Construct empty attribute map
AttributeMap() : std::unordered_map<std::string, OwnedAttributeValue>() {}

// Contruct attribute map and populate with attributes
// Construct attribute map and populate with attributes
AttributeMap(const opentelemetry::common::KeyValueIterable &attributes) : AttributeMap()
{
attributes.ForEachKeyValue(
Expand All @@ -118,6 +118,19 @@ class AttributeMap : public std::unordered_map<std::string, OwnedAttributeValue>
});
}

// Construct attribute map and populate with optional attributes
AttributeMap(const opentelemetry::common::KeyValueIterable *attributes) : AttributeMap()
{
if (attributes != nullptr)
{
attributes->ForEachKeyValue(
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
SetAttribute(key, value);
return true;
Copy link
Member

@lalitb lalitb Sep 26, 2023

Choose a reason for hiding this comment

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

nit - some code duplication, which can be moved to a separate utils method. But don't see it really required to fix

});
}
}

// Construct map from initializer list by applying `SetAttribute` transform for every attribute
AttributeMap(
std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
Expand Down
8 changes: 8 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/meter_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,18 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider
*/
explicit MeterProvider(std::unique_ptr<MeterContext> context) noexcept;

#if OPENTELEMETRY_ABI_VERSION_NO >= 2
nostd::shared_ptr<opentelemetry::metrics::Meter> GetMeter(
nostd::string_view name,
nostd::string_view version = "",
nostd::string_view schema_url = "",
const opentelemetry::common::KeyValueIterable *attributes = nullptr) noexcept override;
#else
nostd::shared_ptr<opentelemetry::metrics::Meter> GetMeter(
nostd::string_view name,
nostd::string_view version = "",
nostd::string_view schema_url = "") noexcept override;
#endif

#ifdef ENABLE_REMOVE_METER_PREVIEW
void RemoveMeter(nostd::string_view name,
Expand Down
19 changes: 16 additions & 3 deletions sdk/src/metrics/meter_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,17 @@ MeterProvider::MeterProvider(std::unique_ptr<ViewRegistry> views,
nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url) noexcept
nostd::string_view schema_url
#if OPENTELEMETRY_ABI_VERSION_NO >= 2
,
const opentelemetry::common::KeyValueIterable *attributes
#endif
) noexcept
{
#if OPENTELEMETRY_ABI_VERSION_NO < 2
const opentelemetry::common::KeyValueIterable *attributes = nullptr;
#endif

if (name.data() == nullptr || name == "")
{
OTEL_INTERNAL_LOG_WARN("[MeterProvider::GetMeter] Library name is empty.");
Expand All @@ -53,8 +62,12 @@ nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
return nostd::shared_ptr<metrics_api::Meter>{meter};
}
}
auto lib = instrumentationscope::InstrumentationScope::Create(name, version, schema_url);
auto meter = std::shared_ptr<Meter>(new Meter(context_, std::move(lib)));

instrumentationscope::InstrumentationScopeAttributes attrs_map(attributes);
auto scope =
instrumentationscope::InstrumentationScope::Create(name, version, schema_url, attrs_map);

auto meter = std::shared_ptr<Meter>(new Meter(context_, std::move(scope)));
context_->AddMeter(meter);
return nostd::shared_ptr<metrics_api::Meter>{meter};
}
Expand Down