Skip to content

Commit

Permalink
Merge branch 'main' into fix_breaking_getmeter_2033
Browse files Browse the repository at this point in the history
  • Loading branch information
marcalff committed Jul 10, 2023
2 parents d2a107a + 5561393 commit e287db5
Show file tree
Hide file tree
Showing 27 changed files with 234 additions and 40 deletions.
20 changes: 20 additions & 0 deletions .github/workflows/project_management_comment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@

name: Add comment
on:
issues:
types:
- labeled
jobs:
add-comment:
if: github.event.label.name == 'help wanted'
runs-on: ubuntu-latest
permissions:
issues: write
steps:
- name: Add comment
uses: peter-evans/create-or-update-comment@a35cf36e5301d70b76f316e867e7788a55a31dae
with:
issue-number: ${{ github.event.issue.number }}
body: |
This issue is available for anyone to work on. **Make sure to reference this issue in your pull request.**
:sparkles: Thank you for your contribution! :sparkles:
25 changes: 25 additions & 0 deletions .github/workflows/project_management_issue_open.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: OpenTelemetry-cpp project
on:
issues:
types:
- reopened
- opened
pull_request:
types:
- reopened
- opened
jobs:
label_issues:
runs-on: ubuntu-latest
permissions:
issues: write
steps:
- uses: actions/github-script@v6
with:
script: |
github.rest.issues.addLabels({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
labels: ["needs-triage"]
})
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,25 @@ Increment the:
* [API] Remove include_trace_context
[#2194](https://github.com/open-telemetry/opentelemetry-cpp/pull/2194)

* [API] Remove Meters
[#2205](https://github.com/open-telemetry/opentelemetry-cpp/pull/2205)

* [SDK] MeterProvider should own MeterContext, not share it
[#2218](https://github.com/open-telemetry/opentelemetry-cpp/pull/2218)

* [SDK] TracerProvider should own TracerContext, not share it
[#2221](https://github.com/open-telemetry/opentelemetry-cpp/pull/2221)

Important changes:

* [API] Remove Meters
[#2205](https://github.com/open-telemetry/opentelemetry-cpp/pull/2205)
* The CMake option `WITH_REMOVE_METER_PREVIEW` was added.
* This option is experimental, and may change in the future.
* Enabling it is an ABI breaking change.

Breaking changes:

* [REMOVAL] Remove the jaeger exporter
[#2031](https://github.com/open-telemetry/opentelemetry-cpp/pull/2031)
* The CMake `WITH_JAEGER` option has been removed
Expand All @@ -37,6 +51,14 @@ Important changes:
`MeterContext`, instead of a `shared_ptr`.
* Please adjust SDK configuration code accordingly.

* [SDK] TracerProvider should own TracerContext, not share it
[#2221](https://github.com/open-telemetry/opentelemetry-cpp/pull/2221)
* The `TracerProvider` constructor now takes a `unique_ptr` on
`TracerContext`, instead of a `shared_ptr`.
* The `LoggerProvider` constructor now takes a `unique_ptr` on
`LoggerContext`, instead of a `shared_ptr`.
* Please adjust SDK configuration code accordingly.

## [1.9.1] 2023-05-26

* [DEPRECATION] Drop C++11 support
Expand Down
8 changes: 8 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ option(WITH_OPENTRACING "Whether to include the Opentracing shim" OFF)
option(OTELCPP_VERSIONED_LIBS "Whether to generate the versioned shared libs"
OFF)

#
# This option is experimental, subject to change in the spec:
#
# * https://github.com/open-telemetry/opentelemetry-specification/issues/2232
#
option(WITH_REMOVE_METER_PREVIEW
"EXPERIMENTAL, ABI BREAKING: Allow to remove a meter" OFF)

if(OTELCPP_VERSIONED_LIBS AND NOT BUILD_SHARED_LIBS)
message(FATAL_ERROR "OTELCPP_VERSIONED_LIBS=ON requires BUILD_SHARED_LIBS=ON")
endif()
Expand Down
5 changes: 5 additions & 0 deletions api/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ if(WITH_ASYNC_EXPORT_PREVIEW)
target_compile_definitions(opentelemetry_api INTERFACE ENABLE_ASYNC_EXPORT)
endif()

if(WITH_REMOVE_METER_PREVIEW)
target_compile_definitions(opentelemetry_api
INTERFACE ENABLE_REMOVE_METER_PREVIEW)
endif()

# A better place should be in sdk, not api
if(WITH_OTLP_HTTP_SSL_PREVIEW)
target_compile_definitions(opentelemetry_api
Expand Down
6 changes: 6 additions & 0 deletions api/include/opentelemetry/metrics/meter_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ class MeterProvider
nostd::string_view library_version = "",
nostd::string_view schema_url = "") noexcept = 0;
#endif

#ifdef ENABLE_REMOVE_METER_PREVIEW
virtual void RemoveMeter(nostd::string_view library_name,
nostd::string_view library_version = "",
nostd::string_view schema_url = "") noexcept = 0;
#endif
};
} // namespace metrics
OPENTELEMETRY_END_NAMESPACE
7 changes: 7 additions & 0 deletions api/include/opentelemetry/metrics/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ class NoopMeterProvider final : public MeterProvider
}
#endif

#ifdef ENABLE_REMOVE_METER_PREVIEW
void RemoveMeter(nostd::string_view /* name */,
nostd::string_view /* version */,
nostd::string_view /* schema_url */) noexcept override
{}
#endif

private:
nostd::shared_ptr<Meter> meter_;
};
Expand Down
3 changes: 3 additions & 0 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ elif [[ "$1" == "cmake.maintainer.sync.test" ]]; then
-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 \
Expand All @@ -129,6 +130,7 @@ elif [[ "$1" == "cmake.maintainer.async.test" ]]; then
-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 \
Expand All @@ -153,6 +155,7 @@ elif [[ "$1" == "cmake.maintainer.cpp11.async.test" ]]; then
-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 \
Expand Down
4 changes: 2 additions & 2 deletions examples/grpc/tracer_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ void InitTracer()
std::vector<std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor>> processors;
processors.push_back(std::move(processor));
// Default is an always-on sampler.
std::shared_ptr<opentelemetry::sdk::trace::TracerContext> context =
std::unique_ptr<opentelemetry::sdk::trace::TracerContext> context =
opentelemetry::sdk::trace::TracerContextFactory::Create(std::move(processors));
std::shared_ptr<opentelemetry::trace::TracerProvider> provider =
opentelemetry::sdk::trace::TracerProviderFactory::Create(context);
opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(context));
// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);

Expand Down
4 changes: 2 additions & 2 deletions examples/http/tracer_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ void InitTracer()
std::vector<std::unique_ptr<opentelemetry::sdk::trace::SpanProcessor>> processors;
processors.push_back(std::move(processor));
// Default is an always-on sampler.
std::shared_ptr<opentelemetry::sdk::trace::TracerContext> context =
std::unique_ptr<opentelemetry::sdk::trace::TracerContext> context =
opentelemetry::sdk::trace::TracerContextFactory::Create(std::move(processors));
std::shared_ptr<opentelemetry::trace::TracerProvider> provider =
opentelemetry::sdk::trace::TracerProviderFactory::Create(context);
opentelemetry::sdk::trace::TracerProviderFactory::Create(std::move(context));
// Set the global trace provider
opentelemetry::trace::Provider::SetTracerProvider(provider);

Expand Down
7 changes: 4 additions & 3 deletions ext/test/w3c_tracecontext_test/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ void initTracer()
new trace_sdk::SimpleSpanProcessor(std::move(exporter)));
std::vector<std::unique_ptr<trace_sdk::SpanProcessor>> processors;
processors.push_back(std::move(processor));
auto context = std::make_shared<trace_sdk::TracerContext>(std::move(processors));
auto provider =
nostd::shared_ptr<trace_api::TracerProvider>(new trace_sdk::TracerProvider(context));
auto context = std::unique_ptr<trace_sdk::TracerContext>(
new trace_sdk::TracerContext(std::move(processors)));
auto provider = nostd::shared_ptr<trace_api::TracerProvider>(
new trace_sdk::TracerProvider(std::move(context)));
// Set the global trace provider
trace_api::Provider::SetTracerProvider(provider);
}
Expand Down
6 changes: 3 additions & 3 deletions sdk/include/opentelemetry/sdk/logs/logger_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/sdk/logs/processor.h"
# include "opentelemetry/sdk/resource/resource.h"
# include "opentelemetry/version.h"

Expand All @@ -17,7 +18,6 @@ namespace sdk
{
namespace logs
{
class LogRecordProcessor;

/**
* A class which stores the LoggerContext context.
Expand All @@ -40,10 +40,10 @@ class LoggerContext
opentelemetry::sdk::resource::Resource::Create({})) noexcept;

/**
* Attaches a log processor to list of configured processors to this tracer context.
* Attaches a log processor to list of configured processors to this logger context.
* Processor once attached can't be removed.
* @param processor The new log processor for this tracer. This must not be
* a nullptr. Ownership is given to the `TracerContext`.
* a nullptr. Ownership is given to the `LoggerContext`.
*
* Note: This method is not thread safe.
*/
Expand Down
6 changes: 3 additions & 3 deletions sdk/include/opentelemetry/sdk/logs/logger_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider

/**
* Initialize a new logger provider with a specified context
* @param context The shared logger configuration/pipeline for this provider.
* @param context The owned logger configuration/pipeline for this provider.
*/
explicit LoggerProvider(std::shared_ptr<sdk::logs::LoggerContext> context) noexcept;
explicit LoggerProvider(std::unique_ptr<LoggerContext> context) noexcept;

~LoggerProvider() override;

Expand Down Expand Up @@ -106,7 +106,7 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider
private:
// order of declaration is important here - loggers should destroy only after context.
std::vector<std::shared_ptr<opentelemetry::sdk::logs::Logger>> loggers_;
std::shared_ptr<sdk::logs::LoggerContext> context_;
std::shared_ptr<LoggerContext> context_;
std::mutex lock_;
};
} // namespace logs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class OPENTELEMETRY_EXPORT LoggerProviderFactory
* Create a LoggerProvider.
*/
static std::unique_ptr<opentelemetry::logs::LoggerProvider> Create(
std::shared_ptr<sdk::logs::LoggerContext> context);
std::unique_ptr<LoggerContext> context);
};

} // namespace logs
Expand Down
4 changes: 4 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/meter_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ class MeterContext : public std::enable_shared_from_this<MeterContext>
*/
void AddMeter(std::shared_ptr<Meter> meter);

void RemoveMeter(nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url);

/**
* Force all active Collectors to flush any buffered meter data
* within the given timeout.
Expand Down
6 changes: 6 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/meter_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider
nostd::string_view schema_url = "") noexcept override;
#endif

#ifdef ENABLE_REMOVE_METER_PREVIEW
void RemoveMeter(nostd::string_view name,
nostd::string_view version,
nostd::string_view schema_url) noexcept override;
#endif

/**
* Obtain the resource associated with this meter provider.
* @return The resource for this meter provider.
Expand Down
4 changes: 2 additions & 2 deletions sdk/include/opentelemetry/sdk/trace/tracer_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider

/**
* Initialize a new tracer provider with a specified context
* @param context The shared tracer configuration/pipeline for this provider.
* @param context The owned tracer configuration/pipeline for this provider.
*/
explicit TracerProvider(std::shared_ptr<TracerContext> context) noexcept;
explicit TracerProvider(std::unique_ptr<TracerContext> context) noexcept;

~TracerProvider() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class OPENTELEMETRY_EXPORT TracerProviderFactory
/* Create with a tracer context. */

static std::unique_ptr<opentelemetry::trace::TracerProvider> Create(
std::shared_ptr<TracerContext> context);
std::unique_ptr<TracerContext> context);
};

} // namespace trace
Expand Down
14 changes: 6 additions & 8 deletions sdk/src/logs/logger_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,27 @@ LoggerProvider::LoggerProvider(std::unique_ptr<LogRecordProcessor> &&processor,
{
std::vector<std::unique_ptr<LogRecordProcessor>> processors;
processors.emplace_back(std::move(processor));
context_ = std::make_shared<sdk::logs::LoggerContext>(std::move(processors), std::move(resource));
context_ = std::make_shared<LoggerContext>(std::move(processors), std::move(resource));
OTEL_INTERNAL_LOG_DEBUG("[LoggerProvider] LoggerProvider created.");
}

LoggerProvider::LoggerProvider(std::vector<std::unique_ptr<LogRecordProcessor>> &&processors,
opentelemetry::sdk::resource::Resource resource) noexcept
: context_{
std::make_shared<sdk::logs::LoggerContext>(std::move(processors), std::move(resource))}
: context_{std::make_shared<LoggerContext>(std::move(processors), std::move(resource))}
{}

LoggerProvider::LoggerProvider() noexcept
: context_{std::make_shared<sdk::logs::LoggerContext>(
std::vector<std::unique_ptr<LogRecordProcessor>>{})}
: context_{std::make_shared<LoggerContext>(std::vector<std::unique_ptr<LogRecordProcessor>>{})}
{}

LoggerProvider::LoggerProvider(std::shared_ptr<sdk::logs::LoggerContext> context) noexcept
: context_{context}
LoggerProvider::LoggerProvider(std::unique_ptr<LoggerContext> context) noexcept
: context_(std::move(context))
{}

LoggerProvider::~LoggerProvider()
{
// Logger hold the shared pointer to the context. So we can not use destructor of LoggerContext to
// Shutdown and flush all pending recordables when we hasve more than one loggers.These
// Shutdown and flush all pending recordables when we have more than one loggers. These
// recordables may use the raw pointer of instrumentation_scope_ in Logger
if (context_)
{
Expand Down
6 changes: 4 additions & 2 deletions sdk/src/logs/logger_provider_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#ifdef ENABLE_LOGS_PREVIEW

# include "opentelemetry/sdk/logs/logger_provider_factory.h"
# include "opentelemetry/sdk/logs/logger_context.h"
# include "opentelemetry/sdk/logs/logger_provider.h"
# include "opentelemetry/sdk/resource/resource.h"

Expand Down Expand Up @@ -46,9 +47,10 @@ std::unique_ptr<opentelemetry::logs::LoggerProvider> LoggerProviderFactory::Crea
}

std::unique_ptr<opentelemetry::logs::LoggerProvider> LoggerProviderFactory::Create(
std::shared_ptr<sdk::logs::LoggerContext> context)
std::unique_ptr<LoggerContext> context)
{
std::unique_ptr<opentelemetry::logs::LoggerProvider> provider(new LoggerProvider(context));
std::unique_ptr<opentelemetry::logs::LoggerProvider> provider(
new LoggerProvider(std::move(context)));
return provider;
}

Expand Down
Loading

0 comments on commit e287db5

Please sign in to comment.