Skip to content

Commit

Permalink
Add building test without RTTI (#1294)
Browse files Browse the repository at this point in the history
  • Loading branch information
owent committed Apr 7, 2022
1 parent 74ec691 commit 6b87300
Show file tree
Hide file tree
Showing 17 changed files with 226 additions and 91 deletions.
22 changes: 22 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,28 @@ jobs:
- name: run tests
run: ./ci/do_ci.sh bazel.noexcept

bazel_nortti:
name: Bazel nortti
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
submodules: 'recursive'
- name: Mount Bazel Cache
uses: actions/cache@v3
env:
cache-name: bazel_cache
with:
path: /home/runner/.cache/bazel
key: bazel_nortti
- name: setup
run: |
sudo ./ci/setup_thrift.sh dependencies_only
sudo ./ci/setup_ci_environment.sh
sudo ./ci/install_bazelisk.sh
- name: run tests
run: ./ci/do_ci.sh bazel.nortti

bazel_asan:
name: Bazel asan config
runs-on: ubuntu-latest
Expand Down
16 changes: 16 additions & 0 deletions api/include/opentelemetry/common/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,19 @@
#else
# define OPENTELEMETRY_MAYBE_UNUSED
#endif

#ifndef OPENTELEMETRY_RTTI_ENABLED
# if defined(__clang__)
# if __has_feature(cxx_rtti)
# define OPENTELEMETRY_RTTI_ENABLED
# endif
# elif defined(__GNUG__)
# if defined(__GXX_RTTI)
# define OPENTELEMETRY_RTTI_ENABLED
# endif
# elif defined(_MSC_VER)
# if defined(_CPPRTTI)
# define OPENTELEMETRY_RTTI_ENABLED
# endif
# endif
#endif
2 changes: 2 additions & 0 deletions ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ CI tests can be run on docker by invoking the script `./ci/run_docker.sh
* `bazel.legacy.test`: build bazel targets and run tests for the targets meant
to work with older compilers.
* `bazel.noexcept`: build bazel targets and run tests with exceptions disabled.
* `bazel.nortti`: build bazel targets and run tests with runtime type
identification disabled.
* `bazel.asan`: build bazel targets and run tests with AddressSanitizer.
* `bazel.tsan`: build bazel targets and run tests with ThreadSanitizer.
* `bazel.valgrind`: build bazel targets and run tests under the valgrind memory
Expand Down
12 changes: 11 additions & 1 deletion ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ mkdir -p "${BUILD_DIR}"
[ -z "${PLUGIN_DIR}" ] && export PLUGIN_DIR=$HOME/plugin
mkdir -p "${PLUGIN_DIR}"

BAZEL_OPTIONS="--copt=-DENABLE_METRICS_PREVIEW --copt=-DENABLE_LOGS_PREVIEW --copt=-DENABLE_TEST"
BAZEL_OPTIONS="--copt=-DENABLE_LOGS_PREVIEW --copt=-DENABLE_TEST"
# Previous legacy metrics use virtual drive, which can not be used without RTTI
if [[ "$1" != "bazel.nortti" ]]; then
BAZEL_OPTIONS="$BAZEL_OPTIONS --copt=-DENABLE_METRICS_PREVIEW"
fi
BAZEL_TEST_OPTIONS="$BAZEL_OPTIONS --test_output=errors"

# https://github.com/bazelbuild/bazel/issues/4341
Expand Down Expand Up @@ -221,6 +225,12 @@ elif [[ "$1" == "bazel.noexcept" ]]; then
bazel $BAZEL_STARTUP_OPTIONS build --copt=-fno-exceptions --build_tag_filters=-jaeger $BAZEL_OPTIONS -- //... -//exporters/prometheus/... -//exporters/jaeger/...
bazel $BAZEL_STARTUP_OPTIONS test --copt=-fno-exceptions --build_tag_filters=-jaeger $BAZEL_TEST_OPTIONS -- //... -//exporters/prometheus/... -//exporters/jaeger/...
exit 0
elif [[ "$1" == "bazel.nortti" ]]; then
# there are some exceptions and error handling code from the Prometheus and Jaeger Clients
# that make this test always fail. ignore Prometheus and Jaeger exporters in the noexcept here.
bazel $BAZEL_STARTUP_OPTIONS build --cxxopt=-fno-rtti --build_tag_filters=-jaeger $BAZEL_OPTIONS -- //... -//exporters/prometheus/... -//exporters/jaeger/...
bazel $BAZEL_STARTUP_OPTIONS test --cxxopt=-fno-rtti --build_tag_filters=-jaeger $BAZEL_TEST_OPTIONS -- //... -//exporters/prometheus/... -//exporters/jaeger/...
exit 0
elif [[ "$1" == "bazel.asan" ]]; then
bazel $BAZEL_STARTUP_OPTIONS test --config=asan $BAZEL_TEST_OPTIONS //...
exit 0
Expand Down
12 changes: 6 additions & 6 deletions examples/multi_processor/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,19 @@ namespace trace_api = opentelemetry::trace;
namespace trace_sdk = opentelemetry::sdk::trace;
namespace nostd = opentelemetry::nostd;

InMemorySpanExporter *memory_span_exporter;

namespace
{
void initTracer()
InMemorySpanExporter *initTracer()
{
auto exporter1 = std::unique_ptr<trace_sdk::SpanExporter>(
new opentelemetry::exporter::trace::OStreamSpanExporter);
auto processor1 = std::unique_ptr<trace_sdk::SpanProcessor>(
new trace_sdk::SimpleSpanProcessor(std::move(exporter1)));

auto exporter2 = std::unique_ptr<trace_sdk::SpanExporter>(new InMemorySpanExporter());
InMemorySpanExporter *memory_span_exporter = new InMemorySpanExporter();
auto exporter2 = std::unique_ptr<trace_sdk::SpanExporter>(memory_span_exporter);

// fetch the exporter for dumping data later
memory_span_exporter = dynamic_cast<InMemorySpanExporter *>(exporter2.get());

auto processor2 = std::unique_ptr<trace_sdk::SpanProcessor>(
new trace_sdk::SimpleSpanProcessor(std::move(exporter2)));
Expand All @@ -44,6 +42,8 @@ void initTracer()
provider->AddProcessor(std::move(processor2));
// Set the global trace provider
trace_api::Provider::SetTracerProvider(std::move(provider));

return memory_span_exporter;
}

void dumpSpans(std::vector<std::unique_ptr<trace_sdk::SpanData>> &spans)
Expand Down Expand Up @@ -81,7 +81,7 @@ void dumpSpans(std::vector<std::unique_ptr<trace_sdk::SpanData>> &spans)
int main()
{
// Removing this line will leave the default noop TracerProvider in place.
initTracer();
InMemorySpanExporter *memory_span_exporter = initTracer();

foo_library();
auto memory_spans = memory_span_exporter->GetData()->GetSpans();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class Logger : public opentelemetry::logs::Logger
common::SystemTimestamp timestamp) noexcept override
{

# ifdef RTTI_ENABLED
# ifdef OPENTELEMETRY_RTTI_ENABLED
common::KeyValueIterable &attribs = const_cast<common::KeyValueIterable &>(attributes);
Properties *evt = dynamic_cast<Properties *>(&attribs);
// Properties *res = dynamic_cast<Properties *>(&resr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ class Tracer : public opentelemetry::trace::Tracer
const opentelemetry::trace::SpanContextKeyValueIterable &links,
const opentelemetry::trace::StartSpanOptions &options = {}) noexcept override
{
#ifdef RTTI_ENABLED
#ifdef OPENTELEMETRY_RTTI_ENABLED
common::KeyValueIterable &attribs = const_cast<common::KeyValueIterable &>(attributes);
Properties *evt = dynamic_cast<Properties *>(&attribs);
if (evt != nullptr)
Expand Down Expand Up @@ -531,7 +531,7 @@ class Tracer : public opentelemetry::trace::Tracer
common::SystemTimestamp timestamp,
const common::KeyValueIterable &attributes) noexcept
{
#ifdef RTTI_ENABLED
#ifdef OPENTELEMETRY_RTTI_ENABLED
common::KeyValueIterable &attribs = const_cast<common::KeyValueIterable &>(attributes);
Properties *evt = dynamic_cast<Properties *>(&attribs);
if (evt != nullptr)
Expand Down
17 changes: 1 addition & 16 deletions exporters/etw/include/opentelemetry/exporters/etw/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <sstream>
#include <string>

#include "opentelemetry/common/macros.h"
#include "opentelemetry/exporters/etw/uuid.h"
#include "opentelemetry/version.h"

Expand All @@ -25,22 +26,6 @@
# pragma comment(lib, "Ole32.Lib")
#endif

#ifndef RTTI_ENABLED
# if defined(__clang__)
# if __has_feature(cxx_rtti)
# define RTTI_ENABLED
# endif
# elif defined(__GNUG__)
# if defined(__GXX_RTTI)
# define RTTI_ENABLED
# endif
# elif defined(_MSC_VER)
# if defined(_CPPRTTI)
# define RTTI_ENABLED
# endif
# endif
#endif

OPENTELEMETRY_BEGIN_NAMESPACE

namespace utils
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class InMemorySpanExporter final : public opentelemetry::sdk::trace::SpanExporte
for (auto &recordable : recordables)
{
auto span = std::unique_ptr<sdk::trace::SpanData>(
dynamic_cast<sdk::trace::SpanData *>(recordable.release()));
static_cast<sdk::trace::SpanData *>(recordable.release()));
if (span != nullptr)
{
data_->Add(std::move(span));
Expand Down
6 changes: 6 additions & 0 deletions sdk/include/opentelemetry/sdk/_metrics/controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
# include <sstream>
# include <thread>
# include <vector>

# include "opentelemetry/_metrics/instrument.h"
# include "opentelemetry/common/macros.h"
# include "opentelemetry/nostd/unique_ptr.h"
# include "opentelemetry/sdk/_metrics/exporter.h"
# include "opentelemetry/sdk/_metrics/meter.h"
Expand Down Expand Up @@ -120,7 +122,11 @@ class PushController
void tick()
{
this->mu_.lock();
# ifdef OPENTELEMETRY_RTTI_ENABLED
std::vector<Record> collected = dynamic_cast<Meter *>(meter_.get())->Collect();
# else
std::vector<Record> collected = static_cast<Meter *>(meter_.get())->Collect();
# endif
for (const auto &rec : collected)
{
processor_->process(rec);
Expand Down
14 changes: 14 additions & 0 deletions sdk/include/opentelemetry/sdk/_metrics/sync_instruments.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
# include <sstream>
# include <stdexcept>
# include <vector>

# include "opentelemetry/_metrics/sync_instruments.h"
# include "opentelemetry/common/macros.h"
# include "opentelemetry/sdk/_metrics/aggregator/counter_aggregator.h"
# include "opentelemetry/sdk/_metrics/aggregator/min_max_sum_count_aggregator.h"
# include "opentelemetry/sdk/_metrics/instrument.h"
Expand Down Expand Up @@ -156,7 +158,11 @@ class Counter final : public SynchronousInstrument<T>, public opentelemetry::met
{
toDelete.push_back(x.first);
}
# ifdef OPENTELEMETRY_RTTI_ENABLED
auto agg_ptr = dynamic_cast<BoundCounter<T> *>(x.second.get())->GetAggregator();
# else
auto agg_ptr = static_cast<BoundCounter<T> *>(x.second.get())->GetAggregator();
# endif
if (agg_ptr->is_updated())
{
agg_ptr->checkpoint();
Expand Down Expand Up @@ -287,7 +293,11 @@ class UpDownCounter final : public SynchronousInstrument<T>,
{
toDelete.push_back(x.first);
}
# ifdef OPENTELEMETRY_RTTI_ENABLED
auto agg_ptr = dynamic_cast<BoundUpDownCounter<T> *>(x.second.get())->GetAggregator();
# else
auto agg_ptr = static_cast<BoundUpDownCounter<T> *>(x.second.get())->GetAggregator();
# endif
if (agg_ptr->is_updated())
{
agg_ptr->checkpoint();
Expand Down Expand Up @@ -417,7 +427,11 @@ class ValueRecorder final : public SynchronousInstrument<T>,
{
toDelete.push_back(x.first);
}
# ifdef OPENTELEMETRY_RTTI_ENABLED
auto agg_ptr = dynamic_cast<BoundValueRecorder<T> *>(x.second.get())->GetAggregator();
# else
auto agg_ptr = static_cast<BoundValueRecorder<T> *>(x.second.get())->GetAggregator();
# endif
if (agg_ptr->is_updated())
{
agg_ptr->checkpoint();
Expand Down
Loading

1 comment on commit 6b87300

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 6b87300 Previous: 74ec691 Ratio
BM_BaselineBuffer/2 7604609.727859497 ns/iter 2404387.950897217 ns/iter 3.16

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.