Skip to content

Commit 758e3a5

Browse files
DCchoudhury15DCchoudhury15
committed
fix: address review feedback for composable samplers YAML parsing
Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>
1 parent 4cf8d39 commit 758e3a5

10 files changed

+198
-75
lines changed

‎sdk/include/opentelemetry/sdk/configuration/composable_parent_threshold_sampler_configuration.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
#pragma once
5-
5+
#include <memory>
66
#include "opentelemetry/sdk/configuration/sampler_configuration.h"
77
#include "opentelemetry/sdk/configuration/sampler_configuration_visitor.h"
88
#include "opentelemetry/version.h"
@@ -12,15 +12,13 @@ namespace sdk
1212
{
1313
namespace configuration
1414
{
15-
1615
class ComposableParentThresholdSamplerConfiguration : public SamplerConfiguration
1716
{
1817
public:
1918
ComposableParentThresholdSamplerConfiguration() = default;
20-
19+
std::unique_ptr<SamplerConfiguration> root;
2120
void Accept(SamplerConfigurationVisitor *visitor) const override;
2221
};
23-
2422
} // namespace configuration
2523
} // namespace sdk
2624
OPENTELEMETRY_END_NAMESPACE

‎sdk/include/opentelemetry/sdk/configuration/composable_probability_sampler_configuration.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ class ComposableProbabilitySamplerConfiguration : public SamplerConfiguration
1717
{
1818
public:
1919
ComposableProbabilitySamplerConfiguration() = default;
20-
21-
double probability{1.0};
22-
20+
double ratio{1.0};
2321
void Accept(SamplerConfigurationVisitor *visitor) const override;
2422
};
2523

‎sdk/include/opentelemetry/sdk/configuration/composable_rule_based_sampler_configuration.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
#pragma once
5-
5+
#include <memory>
6+
#include <vector>
7+
#include "opentelemetry/sdk/configuration/composable_rule_based_sampler_rule_configuration.h"
68
#include "opentelemetry/sdk/configuration/sampler_configuration.h"
79
#include "opentelemetry/sdk/configuration/sampler_configuration_visitor.h"
810
#include "opentelemetry/version.h"
@@ -17,7 +19,7 @@ class ComposableRuleBasedSamplerConfiguration : public SamplerConfiguration
1719
{
1820
public:
1921
ComposableRuleBasedSamplerConfiguration() = default;
20-
22+
std::vector<std::unique_ptr<ComposableRuleBasedSamplerRuleConfiguration>> rules;
2123
void Accept(SamplerConfigurationVisitor *visitor) const override;
2224
};
2325

‎sdk/include/opentelemetry/sdk/configuration/composable_rule_based_sampler_rule_attribute_patterns_configuration.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33
#pragma once
4-
#include "opentelemetry/sdk/configuration/sampler_configuration.h"
5-
#include "opentelemetry/sdk/configuration/sampler_configuration_visitor.h"
4+
#include <string>
5+
#include <vector>
66
#include "opentelemetry/version.h"
77
OPENTELEMETRY_BEGIN_NAMESPACE
88
namespace sdk
@@ -12,8 +12,10 @@ namespace configuration
1212
class ComposableRuleBasedSamplerRuleAttributePatternsConfiguration
1313
{
1414
public:
15-
ComposableRuleBasedSamplerRuleAttributePatternsConfiguration() = default;
16-
~ComposableRuleBasedSamplerRuleAttributePatternsConfiguration() = default;
15+
ComposableRuleBasedSamplerRuleAttributePatternsConfiguration() = default;
16+
std::string key;
17+
std::vector<std::string> included;
18+
std::vector<std::string> excluded;
1719
};
1820
} // namespace configuration
1921
} // namespace sdk

‎sdk/include/opentelemetry/sdk/configuration/composable_rule_based_sampler_rule_attribute_values_configuration.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33
#pragma once
4-
#include "opentelemetry/sdk/configuration/sampler_configuration.h"
5-
#include "opentelemetry/sdk/configuration/sampler_configuration_visitor.h"
4+
#include <string>
5+
#include <vector>
66
#include "opentelemetry/version.h"
77
OPENTELEMETRY_BEGIN_NAMESPACE
88
namespace sdk
@@ -12,8 +12,9 @@ namespace configuration
1212
class ComposableRuleBasedSamplerRuleAttributeValuesConfiguration
1313
{
1414
public:
15-
ComposableRuleBasedSamplerRuleAttributeValuesConfiguration() = default;
16-
~ComposableRuleBasedSamplerRuleAttributeValuesConfiguration() = default;
15+
ComposableRuleBasedSamplerRuleAttributeValuesConfiguration() = default;
16+
std::string key;
17+
std::vector<std::string> values;
1718
};
1819
} // namespace configuration
1920
} // namespace sdk

‎sdk/include/opentelemetry/sdk/configuration/composable_rule_based_sampler_rule_configuration.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33
#pragma once
4+
#include <memory>
5+
#include <string>
6+
#include <vector>
7+
#include "opentelemetry/sdk/configuration/composable_rule_based_sampler_rule_attribute_patterns_configuration.h"
8+
#include "opentelemetry/sdk/configuration/composable_rule_based_sampler_rule_attribute_values_configuration.h"
49
#include "opentelemetry/sdk/configuration/sampler_configuration.h"
5-
#include "opentelemetry/sdk/configuration/sampler_configuration_visitor.h"
610
#include "opentelemetry/version.h"
711
OPENTELEMETRY_BEGIN_NAMESPACE
812
namespace sdk
@@ -12,8 +16,13 @@ namespace configuration
1216
class ComposableRuleBasedSamplerRuleConfiguration
1317
{
1418
public:
15-
ComposableRuleBasedSamplerRuleConfiguration() = default;
16-
~ComposableRuleBasedSamplerRuleConfiguration() = default;
19+
ComposableRuleBasedSamplerRuleConfiguration() = default;
20+
21+
std::unique_ptr<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration> attribute_values;
22+
std::unique_ptr<ComposableRuleBasedSamplerRuleAttributePatternsConfiguration> attribute_patterns;
23+
std::vector<std::string> parent;
24+
std::vector<std::string> span_kinds;
25+
std::unique_ptr<SamplerConfiguration> sampler;
1726
};
1827
} // namespace configuration
1928
} // namespace sdk

‎sdk/include/opentelemetry/sdk/configuration/composable_sampler_configuration.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33
#pragma once
4+
#include <memory>
45
#include "opentelemetry/sdk/configuration/sampler_configuration.h"
56
#include "opentelemetry/sdk/configuration/sampler_configuration_visitor.h"
67
#include "opentelemetry/version.h"
@@ -13,6 +14,7 @@ class ComposableSamplerConfiguration : public SamplerConfiguration
1314
{
1415
public:
1516
ComposableSamplerConfiguration() = default;
17+
std::unique_ptr<SamplerConfiguration> inner;
1618
void Accept(SamplerConfigurationVisitor *visitor) const override;
1719
};
1820
} // namespace configuration

‎sdk/src/configuration/configuration_parser.cc‎

Lines changed: 127 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,34 +1712,147 @@ ConfigurationParser::ParseComposableProbabilitySamplerConfiguration(
17121712
const std::unique_ptr<DocumentNode> &node,
17131713
size_t /* depth */) const
17141714
{
1715-
auto model = std::make_unique<ComposableProbabilitySamplerConfiguration>();
1716-
model->probability = node->GetDouble("probability", 1.0);
1715+
auto model = std::make_unique<ComposableProbabilitySamplerConfiguration>();
1716+
model->ratio = node->GetDouble("ratio", 1.0);
17171717
return model;
17181718
}
17191719

17201720
std::unique_ptr<ComposableParentThresholdSamplerConfiguration>
17211721
ConfigurationParser::ParseComposableParentThresholdSamplerConfiguration(
1722-
const std::unique_ptr<DocumentNode> & /* node */,
1723-
size_t /* depth */) const
1722+
const std::unique_ptr<DocumentNode> &node,
1723+
size_t depth) const
17241724
{
1725-
return std::make_unique<ComposableParentThresholdSamplerConfiguration>();
1725+
auto model = std::make_unique<ComposableParentThresholdSamplerConfiguration>();
1726+
std::unique_ptr<DocumentNode> child;
1727+
1728+
child = node->GetChildNode("root");
1729+
if (child)
1730+
{
1731+
model->root = ParseComposableSamplerConfiguration(child, depth + 1);
1732+
}
1733+
return model;
17261734
}
17271735

17281736
std::unique_ptr<ComposableRuleBasedSamplerConfiguration>
17291737
ConfigurationParser::ParseComposableRuleBasedSamplerConfiguration(
1730-
const std::unique_ptr<DocumentNode> & /* node */,
1731-
size_t /* depth */) const
1738+
const std::unique_ptr<DocumentNode> &node,
1739+
size_t depth) const
17321740
{
1733-
// FIXME-CONFIG: Parse rules list from YAML node here
1734-
return std::make_unique<ComposableRuleBasedSamplerConfiguration>();
1741+
auto model = std::make_unique<ComposableRuleBasedSamplerConfiguration>();
1742+
1743+
std::unique_ptr<DocumentNode> rules_node = node->GetChildNode("rules");
1744+
if (rules_node)
1745+
{
1746+
for (auto it = rules_node->begin(); it != rules_node->end(); ++it)
1747+
{
1748+
std::unique_ptr<DocumentNode> rule_node(*it);
1749+
auto rule = std::make_unique<ComposableRuleBasedSamplerRuleConfiguration>();
1750+
1751+
// parse attribute_values
1752+
std::unique_ptr<DocumentNode> av = rule_node->GetChildNode("attribute_values");
1753+
if (av)
1754+
{
1755+
auto avc = std::make_unique<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration>();
1756+
avc->key = av->GetString("key", "");
1757+
auto vals = av->GetChildNode("values");
1758+
if (vals)
1759+
{
1760+
for (auto vit = vals->begin(); vit != vals->end(); ++vit)
1761+
{
1762+
std::unique_ptr<DocumentNode> v(*vit);
1763+
avc->values.push_back(v->AsString());
1764+
}
1765+
}
1766+
rule->attribute_values = std::move(avc);
1767+
}
1768+
1769+
// parse attribute_patterns
1770+
std::unique_ptr<DocumentNode> ap = rule_node->GetChildNode("attribute_patterns");
1771+
if (ap)
1772+
{
1773+
auto apc = std::make_unique<ComposableRuleBasedSamplerRuleAttributePatternsConfiguration>();
1774+
apc->key = ap->GetString("key", "");
1775+
auto included = ap->GetChildNode("included");
1776+
if (included)
1777+
{
1778+
for (auto iit = included->begin(); iit != included->end(); ++iit)
1779+
{
1780+
std::unique_ptr<DocumentNode> i(*iit);
1781+
apc->included.push_back(i->AsString());
1782+
}
1783+
}
1784+
auto excluded = ap->GetChildNode("excluded");
1785+
if (excluded)
1786+
{
1787+
for (auto eit = excluded->begin(); eit != excluded->end(); ++eit)
1788+
{
1789+
std::unique_ptr<DocumentNode> e(*eit);
1790+
apc->excluded.push_back(e->AsString());
1791+
}
1792+
}
1793+
rule->attribute_patterns = std::move(apc);
1794+
}
1795+
1796+
// parse parent list
1797+
std::unique_ptr<DocumentNode> parent = rule_node->GetChildNode("parent");
1798+
if (parent)
1799+
{
1800+
for (auto pit = parent->begin(); pit != parent->end(); ++pit)
1801+
{
1802+
std::unique_ptr<DocumentNode> p(*pit);
1803+
rule->parent.push_back(p->AsString());
1804+
}
1805+
}
1806+
1807+
// parse span_kinds list
1808+
std::unique_ptr<DocumentNode> span_kinds = rule_node->GetChildNode("span_kinds");
1809+
if (span_kinds)
1810+
{
1811+
for (auto kit = span_kinds->begin(); kit != span_kinds->end(); ++kit)
1812+
{
1813+
std::unique_ptr<DocumentNode> k(*kit);
1814+
rule->span_kinds.push_back(k->AsString());
1815+
}
1816+
}
1817+
1818+
// parse the rule's sampler
1819+
std::unique_ptr<DocumentNode> sampler = rule_node->GetChildNode("sampler");
1820+
if (sampler)
1821+
{
1822+
rule->sampler = ParseComposableSamplerConfiguration(sampler, depth + 1);
1823+
}
1824+
1825+
model->rules.push_back(std::move(rule));
1826+
}
1827+
}
1828+
1829+
return model;
17351830
}
17361831

17371832
std::unique_ptr<ComposableSamplerConfiguration>
1738-
ConfigurationParser::ParseComposableSamplerConfiguration(
1739-
const std::unique_ptr<DocumentNode> & /* node */,
1740-
size_t /* depth */) const
1833+
ConfigurationParser::ParseComposableSamplerConfiguration(const std::unique_ptr<DocumentNode> &node,
1834+
size_t depth) const
17411835
{
1742-
return std::make_unique<ComposableSamplerConfiguration>();
1836+
auto model = std::make_unique<ComposableSamplerConfiguration>();
1837+
1838+
for (auto it = node->begin_properties(); it != node->end_properties(); ++it)
1839+
{
1840+
std::string inner_name = it.Name();
1841+
auto inner_child = it.Value();
1842+
1843+
if (inner_name == "always_off")
1844+
model->inner = ParseComposableAlwaysOffSamplerConfiguration(inner_child, depth);
1845+
else if (inner_name == "always_on")
1846+
model->inner = ParseComposableAlwaysOnSamplerConfiguration(inner_child, depth);
1847+
else if (inner_name == "probability")
1848+
model->inner = ParseComposableProbabilitySamplerConfiguration(inner_child, depth);
1849+
else if (inner_name == "parent_threshold")
1850+
model->inner = ParseComposableParentThresholdSamplerConfiguration(inner_child, depth);
1851+
else if (inner_name == "rule_based")
1852+
model->inner = ParseComposableRuleBasedSamplerConfiguration(inner_child, depth);
1853+
}
1854+
1855+
return model;
17431856
}
17441857

17451858
std::unique_ptr<ExtensionSamplerConfiguration>
@@ -1812,27 +1925,7 @@ std::unique_ptr<SamplerConfiguration> ConfigurationParser::ParseSamplerConfigura
18121925
{
18131926
model = ParseTraceIdRatioBasedSamplerConfiguration(child, depth);
18141927
}
1815-
else if (name == "composable_always_off")
1816-
{
1817-
model = ParseComposableAlwaysOffSamplerConfiguration(child, depth);
1818-
}
1819-
else if (name == "composable_always_on")
1820-
{
1821-
model = ParseComposableAlwaysOnSamplerConfiguration(child, depth);
1822-
}
1823-
else if (name == "composable_probability")
1824-
{
1825-
model = ParseComposableProbabilitySamplerConfiguration(child, depth);
1826-
}
1827-
else if (name == "composable_parent_threshold")
1828-
{
1829-
model = ParseComposableParentThresholdSamplerConfiguration(child, depth);
1830-
}
1831-
else if (name == "composable_rule_based")
1832-
{
1833-
model = ParseComposableRuleBasedSamplerConfiguration(child, depth);
1834-
}
1835-
else if (name == "composable")
1928+
else if (name == "composite/development")
18361929
{
18371930
model = ParseComposableSamplerConfiguration(child, depth);
18381931
}

‎sdk/src/configuration/sdk_builder.cc‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,7 @@ class SamplerBuilder : public opentelemetry::sdk::configuration::SamplerConfigur
390390
const opentelemetry::sdk::configuration::ComposableProbabilitySamplerConfiguration *model)
391391
override
392392
{
393-
sampler =
394-
opentelemetry::sdk::trace::TraceIdRatioBasedSamplerFactory::Create(model->probability);
393+
sampler = opentelemetry::sdk::trace::TraceIdRatioBasedSamplerFactory::Create(model->ratio);
395394
}
396395

397396
void VisitComposableParentThreshold(
@@ -410,11 +409,18 @@ class SamplerBuilder : public opentelemetry::sdk::configuration::SamplerConfigur
410409
sampler = opentelemetry::sdk::trace::AlwaysOnSamplerFactory::Create();
411410
}
412411

413-
void VisitComposable(const opentelemetry::sdk::configuration::ComposableSamplerConfiguration
414-
* /* model */) override
412+
void VisitComposable(
413+
const opentelemetry::sdk::configuration::ComposableSamplerConfiguration *model) override
415414
{
416-
OTEL_INTERNAL_LOG_WARN("ComposableSampler wrapper not yet fully supported by SDK");
417-
sampler = opentelemetry::sdk::trace::AlwaysOnSamplerFactory::Create();
415+
if (model->inner)
416+
{
417+
model->inner->Accept(this);
418+
}
419+
else
420+
{
421+
OTEL_INTERNAL_LOG_WARN("ComposableSampler: no inner sampler configured");
422+
sampler = opentelemetry::sdk::trace::AlwaysOnSamplerFactory::Create();
423+
}
418424
}
419425

420426
std::unique_ptr<opentelemetry::sdk::trace::Sampler> sampler;

0 commit comments

Comments
 (0)