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 all 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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ Increment the:
[#2324](https://github.com/open-telemetry/opentelemetry-cpp/pull/2326)
* [EXPORTER] Replace colons with underscores when converting to Prometheus label
[#2324](https://github.com/open-telemetry/opentelemetry-cpp/pull/2330)
* [API] Add InstrumentationScope attributes in MeterProvider::GetMeter()
[#2224](https://github.com/open-telemetry/opentelemetry-cpp/pull/2224)

Important changes:

* [API] Add InstrumentationScope attributes in MeterProvider::GetMeter()
[#2224](https://github.com/open-telemetry/opentelemetry-cpp/pull/2224)
* MeterProvider::GetMeter() now accepts InstrumentationScope attributes.
* Because this is an `ABI` breaking change, the fix is only available
with the `CMake` option `WITH_ABI_VERSION_2=ON`.
* When building with `CMake` option `WITH_ABI_VERSION_1=ON` (by default)
the `ABI` is unchanged, and the fix is not available.

Breaking changes:

Expand Down
115 changes: 106 additions & 9 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,19 +23,113 @@ class MeterProvider
{
public:
virtual ~MeterProvider() = default;

#if OPENTELEMETRY_ABI_VERSION_NO >= 2

/**
* Gets or creates a named Meter instance (ABI).
*
* @since ABI_VERSION 2
*
* @param[in] name Meter instrumentation scope
* @param[in] version Instrumentation scope version
* @param[in] schema_url Instrumentation scope schema URL
* @param[in] attributes Instrumentation scope attributes (optional, may be nullptr)
*/
virtual nostd::shared_ptr<Meter> GetMeter(
nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url,
const common::KeyValueIterable *attributes) noexcept = 0;

/**
* Gets or creates a named Meter instance (API helper).
*
* @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
*/
nostd::shared_ptr<Meter> GetMeter(nostd::string_view name,
nostd::string_view version = "",
nostd::string_view schema_url = "")
{
return GetMeter(name, version, schema_url, nullptr);
}

/**
* Gets or creates a named Meter instance (API helper).
*
* @since ABI_VERSION 2
*
* @param[in] name Meter instrumentation scope
* @param[in] version Instrumentation scope version
* @param[in] schema_url Instrumentation scope schema URL
* @param[in] attributes Instrumentation scope attributes
*/
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)
{
/* Build a container from std::initializer_list. */
nostd::span<const std::pair<nostd::string_view, common::AttributeValue>> attributes_span{
attributes.begin(), attributes.end()};

/* Build a view on the container. */
common::KeyValueIterableView<
nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>>
iterable_attributes{attributes_span};

/* Add attributes using the view. */
return GetMeter(name, version, schema_url, &iterable_attributes);
}

/**
* Gets or creates a named Meter instance (API helper).
*
* @since ABI_VERSION 2
*
* @param[in] name Meter instrumentation scope
* @param[in] version Instrumentation scope version
* @param[in] schema_url Instrumentation scope schema URL
* @param[in] attributes Instrumentation scope attributes container
*/
template <class T,
nostd::enable_if_t<common::detail::is_key_value_iterable<T>::value> * = nullptr>
nostd::shared_ptr<Meter> GetMeter(nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url,
const T &attributes)
{
/* Build a view on the container. */
common::KeyValueIterableView<T> iterable_attributes(attributes);

/* Add attributes using the view. */
return GetMeter(name, version, schema_url, &iterable_attributes);
}
marcalff marked this conversation as resolved.
Show resolved Hide resolved

#else
/**
* Gets or creates a named Meter instance.
* Gets or creates a named Meter instance (ABI)
*
* Optionally a version can be passed to create a named and versioned 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

#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 = "",
nostd::string_view schema_url = "") noexcept = 0;
virtual void RemoveMeter(nostd::string_view name,
nostd::string_view version = "",
nostd::string_view schema_url = "") noexcept = 0;
#endif
};
} // namespace metrics
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
13 changes: 13 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,23 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider
*/
explicit MeterProvider(std::unique_ptr<MeterContext> context) noexcept;

/*
Make sure GetMeter() helpers from the API are seen in overload resolution.
*/
using opentelemetry::metrics::MeterProvider::GetMeter;

#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) 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
20 changes: 18 additions & 2 deletions sdk/src/metrics/meter_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,23 @@ MeterProvider::MeterProvider(std::unique_ptr<ViewRegistry> views,
OTEL_INTERNAL_LOG_DEBUG("[MeterProvider] MeterProvider created.");
}

#if OPENTELEMETRY_ABI_VERSION_NO >= 2
nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url,
const opentelemetry::common::KeyValueIterable *attributes) noexcept
#else
nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url) noexcept
#endif
{
#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 +65,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
Loading