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

stats: allow specifying tags when creating metric objects #9743

Merged
merged 46 commits into from
Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
afacd77
stats: allow specifying tags when creating metric objects
snowp Jan 19, 2020
c58b68c
Merge branch 'master' of github.com:envoyproxy/envoy into set-tags
snowp Jan 19, 2020
6b8ad96
fix MockStore
snowp Jan 19, 2020
369e218
format
snowp Jan 19, 2020
3272179
Merge branch 'master' of github.com:envoyproxy/envoy into set-tags
snowp Jan 20, 2020
54a688a
wip
snowp Jan 22, 2020
8aefabc
wip
snowp Jan 23, 2020
b936383
Merge remote-tracking branch 'envoy/master' into set-tags
snowp Feb 11, 2020
95eb0a1
use stat name tags instead of string names
snowp Feb 12, 2020
14506e6
format
snowp Feb 12, 2020
59b91ae
format
snowp Feb 12, 2020
c219777
format
snowp Feb 12, 2020
1a4d142
format
snowp Feb 12, 2020
91b0c83
fix undefined behavior
snowp Feb 13, 2020
cdac16f
format
snowp Feb 13, 2020
81d8c0e
implement tag overload for test classes
snowp Feb 13, 2020
b238587
format
snowp Feb 13, 2020
a1538c3
format
snowp Feb 13, 2020
efefe70
pass through both stat name and string tags
snowp Feb 16, 2020
ca14d85
pass through stat name tags for gauges/counters
snowp Feb 16, 2020
0767b64
fix test invocations to allocate*
snowp Feb 16, 2020
b80306b
add tests for counters/gauges
snowp Feb 16, 2020
d4041fa
fix metric_impl_test
snowp Feb 16, 2020
23935a0
Merge remote-tracking branch 'envoy/master' into set-tags
snowp Feb 16, 2020
3302d62
fix allocator_impl_test
snowp Feb 16, 2020
ac4eaa8
fix admin_test
snowp Feb 17, 2020
767d462
get rid of std::vector<Tag>s being passed around, use RAII objects fo…
snowp Feb 20, 2020
38e68f8
move impl to .cc, document lifetime
snowp Feb 20, 2020
030422d
typedef std::vector<Tag> -> TagVector
snowp Feb 20, 2020
fb95862
document tag extraction behavior
snowp Feb 20, 2020
655115d
fix admin-test to use stat name tags
snowp Feb 20, 2020
ee33855
do not perform tag extraction for overloads that provide tags
snowp Feb 20, 2020
058186b
remove redundant using statement
snowp Feb 21, 2020
07d8603
use StatNameManagedStorage
snowp Feb 21, 2020
78fd40d
use a single absl::optional overload, some name fixups in tag helper
snowp Feb 21, 2020
675b6f3
add TODO
snowp Feb 21, 2020
759d5b4
format
snowp Feb 21, 2020
02b9456
fix typo
snowp Feb 21, 2020
e92f0ee
pass absl::nullopt to another function call
snowp Feb 21, 2020
7832f91
more call sites
snowp Feb 21, 2020
6a403a2
Merge remote-tracking branch 'envoy/master' into set-tags
snowp Feb 24, 2020
c3f8b5b
provide an overload that doesn't require tags + revert call sites tha…
snowp Feb 25, 2020
e01f7fa
spelling
snowp Feb 25, 2020
07804fe
qualify call to non-virtual func call
snowp Feb 25, 2020
85100c3
rename virtual functions with 'withTags' + other cleanups
snowp Feb 25, 2020
66a4095
format
snowp Feb 25, 2020
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
8 changes: 4 additions & 4 deletions include/envoy/stats/allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,22 @@ class Allocator {
/**
* @param name the full name of the stat.
* @param tag_extracted_name the name of the stat with tag-values stripped out.
* @param tags the extracted tag values.
* @param tags the tag values.
* @return CounterSharedPtr a counter, or nullptr if allocation failed, in which case
* tag_extracted_name and tags are not moved.
*/
virtual CounterSharedPtr makeCounter(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags) PURE;
const StatNameTagVector& stat_name_tags) PURE;

/**
* @param name the full name of the stat.
* @param tag_extracted_name the name of the stat with tag-values stripped out.
* @param tags the extracted tag values.
* @param stat_name_tags the tag values.
* @return GaugeSharedPtr a gauge, or nullptr if allocation failed, in which case
* tag_extracted_name and tags are not moved.
*/
virtual GaugeSharedPtr makeGauge(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags,
const StatNameTagVector& stat_name_tags,
Gauge::ImportMode import_mode) PURE;

virtual const SymbolTable& constSymbolTable() const PURE;
Expand Down
48 changes: 45 additions & 3 deletions include/envoy/stats/scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "envoy/common/pure.h"
#include "envoy/stats/histogram.h"
#include "envoy/stats/symbol_table.h"
#include "envoy/stats/tag.h"

#include "absl/types/optional.h"

Expand Down Expand Up @@ -47,10 +48,22 @@ class Scope {
virtual void deliverHistogramToSinks(const Histogram& histogram, uint64_t value) PURE;

/**
* Creates a Counter from the stat name. Tag extraction will be performed on the name.
* @param name The name of the stat, obtained from the SymbolTable.
* @return a counter within the scope's namespace.
*/
virtual Counter& counterFromStatName(StatName name) PURE;
Counter& counterFromStatName(const StatName& name) {
return counterFromStatNameWithTags(name, absl::nullopt);
}
/**
* Creates a Counter from the stat name and tags. If tags are not provided, tag extraction
* will be performed on the name.
* @param name The name of the stat, obtained from the SymbolTable.
* @param tags optionally specified tags.
* @return a counter within the scope's namespace.
jmarantz marked this conversation as resolved.
Show resolved Hide resolved
*/
virtual Counter& counterFromStatNameWithTags(const StatName& name,
StatNameTagVectorOptRef tags) PURE;

/**
* TODO(#6667): this variant is deprecated: use counterFromStatName.
Expand All @@ -60,11 +73,25 @@ class Scope {
virtual Counter& counter(const std::string& name) PURE;

/**
* Creates a Gauge from the stat name. Tag extraction will be performed on the name.
* @param name The name of the stat, obtained from the SymbolTable.
* @param import_mode Whether hot-restart should accumulate this value.
* @return a gauge within the scope's namespace.
*/
Gauge& gaugeFromStatName(const StatName& name, Gauge::ImportMode import_mode) {
return gaugeFromStatNameWithTags(name, absl::nullopt, import_mode);
}

/**
* Creates a Gauge from the stat name and tags. If tags are not provided, tag extraction
* will be performed on the name.
* @param name The name of the stat, obtained from the SymbolTable.
* @param tags optionally specified tags.
* @param import_mode Whether hot-restart should accumulate this value.
* @return a gauge within the scope's namespace.
*/
virtual Gauge& gaugeFromStatName(StatName name, Gauge::ImportMode import_mode) PURE;
virtual Gauge& gaugeFromStatNameWithTags(const StatName& name, StatNameTagVectorOptRef tags,
Gauge::ImportMode import_mode) PURE;

/**
* TODO(#6667): this variant is deprecated: use gaugeFromStatName.
Expand All @@ -80,11 +107,26 @@ class Scope {
virtual NullGaugeImpl& nullGauge(const std::string& name) PURE;

/**
* Creates a Histogram from the stat name. Tag extraction will be performed on the name.
* @param name The name of the stat, obtained from the SymbolTable.
* @param unit The unit of measurement.
* @return a histogram within the scope's namespace with a particular value type.
*/
Histogram& histogramFromStatName(const StatName& name, Histogram::Unit unit) {
return histogramFromStatNameWithTags(name, absl::nullopt, unit);
}

/**
* Creates a Histogram from the stat name and tags. If tags are not provided, tag extraction
* will be performed on the name.
* @param name The name of the stat, obtained from the SymbolTable.
* @param tags optionally specified tags.
* @param unit The unit of measurement.
* @return a histogram within the scope's namespace with a particular value type.
*/
virtual Histogram& histogramFromStatName(StatName name, Histogram::Unit unit) PURE;
virtual Histogram& histogramFromStatNameWithTags(const StatName& name,
StatNameTagVectorOptRef tags,
Histogram::Unit unit) PURE;

/**
* TODO(#6667): this variant is deprecated: use histogramFromStatName.
Expand Down
4 changes: 2 additions & 2 deletions include/envoy/stats/stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
#include "envoy/common/pure.h"
#include "envoy/stats/refcount_ptr.h"
#include "envoy/stats/symbol_table.h"
#include "envoy/stats/tag.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Stats {

class Allocator;
struct Tag;

/**
* General interface for all stats objects.
Expand All @@ -42,7 +42,7 @@ class Metric : public RefcountInterface {
/**
* Returns a vector of configurable tags to identify this Metric.
*/
virtual std::vector<Tag> tags() const PURE;
virtual TagVector tags() const PURE;

/**
* See a more detailed description in tagExtractedStatName(), which is the
Expand Down
14 changes: 14 additions & 0 deletions include/envoy/stats/tag.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

#include <string>

#include "envoy/stats/symbol_table.h"

#include "absl/types/optional.h"

namespace Envoy {
namespace Stats {

Expand All @@ -11,7 +15,17 @@ namespace Stats {
struct Tag {
std::string name_;
std::string value_;

bool operator==(const Tag& other) const {
return other.name_ == name_ && other.value_ == value_;
};
jmarantz marked this conversation as resolved.
Show resolved Hide resolved
};

using TagVector = std::vector<Tag>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can drop this one now that this statement is also in stats.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like some things include only tag.h and not stats.h, e.g. tag_extractor.h


using StatNameTag = std::pair<StatName, StatName>;
using StatNameTagVector = std::vector<StatNameTag>;
jmarantz marked this conversation as resolved.
Show resolved Hide resolved
using StatNameTagVectorOptRef = absl::optional<std::reference_wrapper<StatNameTagVector>>;

} // namespace Stats
} // namespace Envoy
2 changes: 1 addition & 1 deletion include/envoy/stats/tag_extractor.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class TagExtractor {
* @param remove_characters set of intervals of character-indices to be removed from name.
* @return bool indicates whether a tag was found in the name.
*/
virtual bool extractTag(absl::string_view stat_name, std::vector<Tag>& tags,
virtual bool extractTag(absl::string_view stat_name, TagVector& tags,
IntervalSet<size_t>& remove_characters) const PURE;

/**
Expand Down
4 changes: 2 additions & 2 deletions include/envoy/stats/tag_producer.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ class TagProducer {
* {"vcluster", "bar"}, and return "vhost.vcluster.c1".
*
* @param metric_name std::string a name of Stats::Metric (Counter, Gauge, Histogram).
* @param tags std::vector a set of Stats::Tag.
* @param tags TagVector a set of Stats::Tag.
*/
virtual std::string produceTags(absl::string_view metric_name, std::vector<Tag>& tags) const PURE;
virtual std::string produceTags(absl::string_view metric_name, TagVector& tags) const PURE;
};

using TagProducerPtr = std::unique_ptr<const TagProducer>;
Expand Down
13 changes: 13 additions & 0 deletions source/common/stats/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ envoy_cc_library(
":stats_lib",
":store_impl_lib",
":symbol_table_creator_lib",
":tag_utility_lib",
"//include/envoy/stats:stats_macros",
"//source/common/stats:allocator_lib",
],
Expand All @@ -68,6 +69,17 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "tag_utility_lib",
srcs = ["tag_utility.cc"],
hdrs = ["tag_utility.h"],
deps = [
":symbol_table_lib",
"//include/envoy/stats:stats_interface",
"//include/envoy/stats:symbol_table_interface",
],
)

envoy_cc_library(
name = "null_counter_lib",
hdrs = ["null_counter.h"],
Expand Down Expand Up @@ -232,6 +244,7 @@ envoy_cc_library(
":stats_lib",
":stats_matcher_lib",
":tag_producer_lib",
":tag_utility_lib",
"//include/envoy/thread_local:thread_local_interface",
],
)
Expand Down
22 changes: 12 additions & 10 deletions source/common/stats/allocator_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ void AllocatorImpl::debugPrint() {
template <class BaseClass> class StatsSharedImpl : public MetricImpl<BaseClass> {
public:
StatsSharedImpl(StatName name, AllocatorImpl& alloc, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags)
: MetricImpl<BaseClass>(name, tag_extracted_name, tags, alloc.symbolTable()), alloc_(alloc) {}
const StatNameTagVector& stat_name_tags)
: MetricImpl<BaseClass>(name, tag_extracted_name, stat_name_tags, alloc.symbolTable()),
alloc_(alloc) {}

~StatsSharedImpl() override {
// MetricImpl must be explicitly cleared() before destruction, otherwise it
Expand Down Expand Up @@ -121,8 +122,8 @@ template <class BaseClass> class StatsSharedImpl : public MetricImpl<BaseClass>
class CounterImpl : public StatsSharedImpl<Counter> {
public:
CounterImpl(StatName name, AllocatorImpl& alloc, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags)
: StatsSharedImpl(name, alloc, tag_extracted_name, tags) {}
const StatNameTagVector& stat_name_tags)
: StatsSharedImpl(name, alloc, tag_extracted_name, stat_name_tags) {}

void removeFromSetLockHeld() EXCLUSIVE_LOCKS_REQUIRED(alloc_.mutex_) override {
const size_t count = alloc_.counters_.erase(statName());
Expand All @@ -149,8 +150,8 @@ class CounterImpl : public StatsSharedImpl<Counter> {
class GaugeImpl : public StatsSharedImpl<Gauge> {
public:
GaugeImpl(StatName name, AllocatorImpl& alloc, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags, ImportMode import_mode)
: StatsSharedImpl(name, alloc, tag_extracted_name, tags) {
const StatNameTagVector& stat_name_tags, ImportMode import_mode)
: StatsSharedImpl(name, alloc, tag_extracted_name, stat_name_tags) {
switch (import_mode) {
case ImportMode::Accumulate:
flags_ |= Flags::LogicAccumulate;
Expand Down Expand Up @@ -228,28 +229,29 @@ class GaugeImpl : public StatsSharedImpl<Gauge> {
};

CounterSharedPtr AllocatorImpl::makeCounter(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags) {
const StatNameTagVector& stat_name_tags) {
Thread::LockGuard lock(mutex_);
ASSERT(gauges_.find(name) == gauges_.end());
auto iter = counters_.find(name);
if (iter != counters_.end()) {
return CounterSharedPtr(*iter);
}
auto counter = CounterSharedPtr(new CounterImpl(name, *this, tag_extracted_name, tags));
auto counter = CounterSharedPtr(new CounterImpl(name, *this, tag_extracted_name, stat_name_tags));
counters_.insert(counter.get());
return counter;
}

GaugeSharedPtr AllocatorImpl::makeGauge(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags,
const StatNameTagVector& stat_name_tags,
Gauge::ImportMode import_mode) {
Thread::LockGuard lock(mutex_);
ASSERT(counters_.find(name) == counters_.end());
auto iter = gauges_.find(name);
if (iter != gauges_.end()) {
return GaugeSharedPtr(*iter);
}
auto gauge = GaugeSharedPtr(new GaugeImpl(name, *this, tag_extracted_name, tags, import_mode));
auto gauge =
GaugeSharedPtr(new GaugeImpl(name, *this, tag_extracted_name, stat_name_tags, import_mode));
gauges_.insert(gauge.get());
return gauge;
}
Expand Down
5 changes: 3 additions & 2 deletions source/common/stats/allocator_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ class AllocatorImpl : public Allocator {

// Allocator
CounterSharedPtr makeCounter(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags) override;
const StatNameTagVector& stat_name_tags) override;
GaugeSharedPtr makeGauge(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags, Gauge::ImportMode import_mode) override;
const StatNameTagVector& stat_name_tags,
Gauge::ImportMode import_mode) override;
SymbolTable& symbolTable() override { return symbol_table_; }
const SymbolTable& constSymbolTable() const override { return symbol_table_; }

Expand Down
10 changes: 5 additions & 5 deletions source/common/stats/histogram_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class HistogramStatisticsImpl : public HistogramStatistics, NonCopyable {
class HistogramImplHelper : public MetricImpl<Histogram> {
public:
HistogramImplHelper(StatName name, const std::string& tag_extracted_name,
const std::vector<Tag>& tags, SymbolTable& symbol_table)
: MetricImpl<Histogram>(name, tag_extracted_name, tags, symbol_table) {}
const StatNameTagVector& stat_name_tags, SymbolTable& symbol_table)
: MetricImpl<Histogram>(name, tag_extracted_name, stat_name_tags, symbol_table) {}
HistogramImplHelper(SymbolTable& symbol_table) : MetricImpl<Histogram>(symbol_table) {}

// RefcountInterface
Expand All @@ -69,9 +69,9 @@ class HistogramImplHelper : public MetricImpl<Histogram> {
class HistogramImpl : public HistogramImplHelper {
public:
HistogramImpl(StatName name, Unit unit, Store& parent, const std::string& tag_extracted_name,
const std::vector<Tag>& tags)
: HistogramImplHelper(name, tag_extracted_name, tags, parent.symbolTable()), unit_(unit),
parent_(parent) {}
const StatNameTagVector& stat_name_tags)
: HistogramImplHelper(name, tag_extracted_name, stat_name_tags, parent.symbolTable()),
unit_(unit), parent_(parent) {}
~HistogramImpl() override {
// We must explicitly free the StatName here in order to supply the
// SymbolTable reference. An RAII alternative would be to store a
Expand Down
6 changes: 3 additions & 3 deletions source/common/stats/isolated_store_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ IsolatedStoreImpl::IsolatedStoreImpl(std::unique_ptr<SymbolTable>&& symbol_table
IsolatedStoreImpl::IsolatedStoreImpl(SymbolTable& symbol_table)
: StoreImpl(symbol_table), alloc_(symbol_table),
counters_([this](StatName name) -> CounterSharedPtr {
return alloc_.makeCounter(name, toString(name), std::vector<Tag>());
return alloc_.makeCounter(name, toString(name), StatNameTagVector{});
}),
gauges_([this](StatName name, Gauge::ImportMode import_mode) -> GaugeSharedPtr {
return alloc_.makeGauge(name, toString(name), std::vector<Tag>(), import_mode);
return alloc_.makeGauge(name, toString(name), StatNameTagVector{}, import_mode);
}),
histograms_([this](StatName name, Histogram::Unit unit) -> HistogramSharedPtr {
return HistogramSharedPtr(
new HistogramImpl(name, unit, *this, toString(name), std::vector<Tag>()));
new HistogramImpl(name, unit, *this, toString(name), StatNameTagVector{}));
}),
null_counter_(new NullCounterImpl(symbol_table)),
null_gauge_(new NullGaugeImpl(symbol_table)) {}
Expand Down
20 changes: 15 additions & 5 deletions source/common/stats/isolated_store_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "common/stats/null_gauge.h"
#include "common/stats/store_impl.h"
#include "common/stats/symbol_table_impl.h"
#include "common/stats/tag_utility.h"
#include "common/stats/utility.h"

#include "absl/container/flat_hash_map.h"
Expand Down Expand Up @@ -100,18 +101,27 @@ class IsolatedStoreImpl : public StoreImpl {
explicit IsolatedStoreImpl(SymbolTable& symbol_table);

// Stats::Scope
Counter& counterFromStatName(StatName name) override { return counters_.get(name); }
Counter& counterFromStatNameWithTags(const StatName& name,
StatNameTagVectorOptRef tags) override {
TagUtility::TagStatNameJoiner joiner(name, tags, symbolTable());
Counter& counter = counters_.get(joiner.nameWithTags());
return counter;
}
ScopePtr createScope(const std::string& name) override;
void deliverHistogramToSinks(const Histogram&, uint64_t) override {}
Gauge& gaugeFromStatName(StatName name, Gauge::ImportMode import_mode) override {
Gauge& gauge = gauges_.get(name, import_mode);
Gauge& gaugeFromStatNameWithTags(const StatName& name, StatNameTagVectorOptRef tags,
Gauge::ImportMode import_mode) override {
TagUtility::TagStatNameJoiner joiner(name, tags, symbolTable());
Gauge& gauge = gauges_.get(joiner.nameWithTags(), import_mode);
gauge.mergeImportMode(import_mode);
return gauge;
}
NullCounterImpl& nullCounter() { return *null_counter_; }
NullGaugeImpl& nullGauge(const std::string&) override { return *null_gauge_; }
Histogram& histogramFromStatName(StatName name, Histogram::Unit unit) override {
Histogram& histogram = histograms_.get(name, unit);
Histogram& histogramFromStatNameWithTags(const StatName& name, StatNameTagVectorOptRef tags,
Histogram::Unit unit) override {
TagUtility::TagStatNameJoiner joiner(name, tags, symbolTable());
Histogram& histogram = histograms_.get(joiner.nameWithTags(), unit);
return histogram;
}
CounterOptConstRef findCounter(StatName name) const override { return counters_.find(name); }
Expand Down
Loading