-
Notifications
You must be signed in to change notification settings - Fork 410
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
[ETW Exporter] Tail based sampling support #1780
Changes from all commits
5611451
6477281
fe3435c
cad3fa3
570a78d
3f22da3
28af4e8
1149a4d
fbe86d3
4bc6ebd
a247b67
4cc1f87
2d9d01d
c202c0e
f8eb147
b988c8d
dc2c7ba
e9423ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
#include "opentelemetry/sdk/trace/sampler.h" | ||
#include "opentelemetry/trace/span.h" | ||
|
||
#pragma once | ||
OPENTELEMETRY_BEGIN_NAMESPACE | ||
namespace exporter | ||
{ | ||
namespace etw | ||
{ | ||
|
||
class TailSampler | ||
{ | ||
public: | ||
// convert to etw span if required for getters on span. | ||
// auto etw_span = static_cast<const opentelemetry::exporter::etw::Span*>(&span); | ||
// Decision based on | ||
// Span::GetStatus() | ||
// Span::GetProperties() | ||
// Span::GetContext() | ||
virtual opentelemetry::sdk::trace::SamplingResult ShouldSample( | ||
const opentelemetry::trace::Span &span) noexcept = 0; | ||
}; | ||
|
||
class AlwaysOnTailSampler : public TailSampler | ||
{ | ||
public: | ||
opentelemetry::sdk::trace::SamplingResult ShouldSample( | ||
const opentelemetry::trace::Span &span) noexcept override | ||
{ | ||
return {opentelemetry::sdk::trace::Decision::RECORD_AND_SAMPLE}; | ||
} | ||
}; | ||
|
||
} // namespace etw | ||
} // namespace exporter | ||
OPENTELEMETRY_END_NAMESPACE |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ | |
# include <gtest/gtest.h> | ||
# include <map> | ||
# include <string> | ||
|
||
# include "opentelemetry//sdk/trace/sampler.h" | ||
# include "opentelemetry/exporters/etw/etw_tracer_exporter.h" | ||
# include "opentelemetry/sdk/trace/samplers/always_off.h" | ||
|
@@ -78,6 +77,16 @@ class MockSampler : public sdk::trace::Sampler | |
std::shared_ptr<Sampler> delegate_sampler_; | ||
}; | ||
|
||
class AlwaysOffTailSampler : public TailSampler | ||
{ | ||
public: | ||
opentelemetry::sdk::trace::SamplingResult ShouldSample( | ||
const opentelemetry::trace::Span &span) noexcept override | ||
{ | ||
return {opentelemetry::sdk::trace::Decision::DROP}; | ||
} | ||
}; | ||
|
||
/* clang-format off */ | ||
TEST(ETWTracer, TracerCheck) | ||
{ | ||
|
@@ -459,6 +468,26 @@ TEST(ETWTracer, AlwayOffSampler) | |
EXPECT_EQ(span->GetContext().IsSampled(), false); | ||
} | ||
|
||
TEST(ETWTracer, AlwayOffTailSampler) | ||
{ | ||
std::string providerName = kGlobalProviderName; // supply unique instrumentation name here | ||
std::unique_ptr<sdk::trace::Sampler> always_on{new sdk::trace::AlwaysOnSampler()}; | ||
sdk::trace::IdGenerator *id_generator = new MockIdGenerator(); | ||
std::unique_ptr<TailSampler> always_off_tail{new AlwaysOffTailSampler()}; | ||
exporter::etw::TracerProvider tp | ||
({ | ||
{"enableTraceId", true}, | ||
{"enableSpanId", true}, | ||
{"enableActivityId", true}, | ||
{"enableRelatedActivityId", true}, | ||
{"enableAutoParent", true} | ||
}, | ||
std::move(always_on), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we start a span and test it is indeed dropped? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I was thinking to mock the actual ETW transport layer and do the test. In current design, this is not easy to do, perhaps can be taken separately if it is fine? |
||
std::unique_ptr<sdk::trace::IdGenerator>(id_generator), | ||
std::move(always_off_tail)); | ||
auto tracer = tp.GetTracer(providerName); | ||
} | ||
|
||
TEST(ETWTracer, CustomIdGenerator) | ||
{ | ||
std::string providerName = kGlobalProviderName; // supply unique instrumentation name here | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of making a virtual call to AlwaysOn sampler by default, perhaps check if there is a non-default sampler set, then make the virtual call? In this way, it doesn't introduce extra cost (except the null check) when tail based sampler is not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good comment. Actually we do have virtual method calls for head-sampler, id-generator both in ETW exporter, and the otel-sdk. There is no reported performance reported with them. Seemingly modern compilers can optimize these repeated calls by caching the address, and avoid virtual indirection. Perhaps we can look back to this if there are reported overhead internally and/or by community.