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: Use SymbolTable API for creating and representing stat names. #6161

Merged
merged 52 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
f6d3ec4
Use SymbolTable API for creating and representing stat names.
jmarantz Mar 4, 2019
9e47dfe
Rename counterx to counterFromStatName; ditto for gaugex, histogramx.
jmarantz Mar 4, 2019
f384eeb
avoid extra copy when encoding strings into FakeSymbolTableImpl.
jmarantz Mar 5, 2019
f138212
Use FakeSymbolTable more consistently. Start updating stats.md for sy…
jmarantz Mar 11, 2019
342cb7c
Merge branch 'master' into stats-symtab-interface
jmarantz Mar 14, 2019
52e07df
checkpoint with impl-specific Encoding class to remove a copy that wa…
jmarantz Mar 14, 2019
08c35f5
hacks to use stack allocated arrays for temps to reduce new/delete ca…
jmarantz Mar 14, 2019
94fba4f
Make a separate populateList for Fake table that doesn't bother encod…
jmarantz Mar 14, 2019
c103f22
Use string_view through to the regexes to avoid an extra std::string …
jmarantz Mar 14, 2019
91ec93c
format
jmarantz Mar 14, 2019
c02132c
cleanups.
jmarantz Mar 14, 2019
31c8a61
remove commented out reference to std::cerr, caught by spelling :)
jmarantz Mar 14, 2019
cb9e6f2
Removed no-longer-needed class FakeSymbolTableImpl::Encoding.
jmarantz Mar 14, 2019
944b811
Remove commented out blocks, add 'override' attribute to virtual dtor…
jmarantz Mar 14, 2019
e3a9eae
Remove commented-out code.
jmarantz Mar 15, 2019
8c72c2d
tweaks pulled over from #6293.
jmarantz Mar 15, 2019
26700c8
back out some changes that are not needed.
jmarantz Mar 15, 2019
323b005
backout router changes which are not needed.
jmarantz Mar 15, 2019
9449596
Remove more changes that are not clearly needed yet in this PR.
jmarantz Mar 15, 2019
61ea4e2
patches from #6299
jmarantz Mar 19, 2019
e6c4ea4
Merge branch 'master' into stats-symtab-interface
jmarantz Mar 20, 2019
38acb61
Merge branch 'master' into stats-symtab-interface
jmarantz Mar 20, 2019
1abc30c
Go back to making SharedStatNameStorageSet a class to assert we've ca…
jmarantz Mar 20, 2019
4f92343
Merge branch 'master' into stats-symtab-interface
jmarantz Mar 20, 2019
4c2eeb2
Update to more aggressive expected memory usage.
jmarantz Mar 28, 2019
a1f3590
Merge branch 'master' into stats-symtab-interface
jmarantz Mar 28, 2019
c16f5c1
fix memory-use limit, remove commented-out block.
jmarantz Mar 28, 2019
48582c9
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 2, 2019
885ee77
Describe the strategy for converting to symbol tables.
jmarantz Apr 2, 2019
292f925
fix links in doc, add deprecation comments to Scope interface, and re…
jmarantz Apr 2, 2019
330b3e7
fix section syntax
jmarantz Apr 2, 2019
06427a2
fix grammar & make other minor edits.
jmarantz Apr 2, 2019
78de4ef
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 2, 2019
ec750f2
remove IsolatedStats*::clear()
jmarantz Apr 4, 2019
ec01f8e
Don't use shared_ptr for the remembered disallowed stats.
jmarantz Apr 6, 2019
2e47198
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 7, 2019
ccc666b
scope_prefixer.cc should go back to using StatName natively.
jmarantz Apr 7, 2019
ca2d3ff
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 8, 2019
57edaad
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 19, 2019
68bf314
Address review comments.
jmarantz Apr 20, 2019
1a704ae
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 20, 2019
a76248a
pull over test cleanup from #6504
jmarantz Apr 21, 2019
e74fe2a
spelling
jmarantz Apr 21, 2019
9eeea11
spelling pedantry
jmarantz Apr 21, 2019
9caab6a
OK it's impossible to put usernames in comments due to pedantic spell…
jmarantz Apr 21, 2019
1655d92
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 22, 2019
e12810b
fix comment.
jmarantz Apr 22, 2019
30b188e
Add more comments to help clarify review comments.
jmarantz Apr 24, 2019
81b9cef
Merge branch 'master' into stats-symtab-interface
jmarantz Apr 24, 2019
8183533
Rename StatNameTempStorage to StatNameManagedStorage and use it in Hy…
jmarantz Apr 25, 2019
ead848f
Improve commenting for StatNameManagedStorage as that's what we expec…
jmarantz Apr 25, 2019
18ae590
delete commented-out code.
jmarantz Apr 26, 2019
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
6 changes: 3 additions & 3 deletions include/envoy/stats/scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Scope {
virtual Counter& counterFromStatName(StatName name) PURE;

/**
* TODO(jmarantz): this variant is deprecated: use counterFromStatName.
* TODO(#6667): this variant is deprecated: use counterFromStatName.
* @param name The name, expressed as a string.
* @return a counter within the scope's namespace.
*/
Expand All @@ -62,7 +62,7 @@ class Scope {
virtual Gauge& gaugeFromStatName(StatName name) PURE;

/**
* TODO(jmarantz): this variant is deprecated: use gaugeFromStatName.
* TODO(#6667): this variant is deprecated: use gaugeFromStatName.
* @param name The name, expressed as a string.
* @return a gauge within the scope's namespace.
*/
Expand All @@ -80,7 +80,7 @@ class Scope {
virtual Histogram& histogramFromStatName(StatName name) PURE;

/**
* TODO(jmarantz): this variant is deprecated: use histogramFromStatName.
* TODO(#6667): this variant is deprecated: use histogramFromStatName.
* @param name The name, expressed as a string.
* @return a histogram within the scope's namespace with a particular value type.
*/
Expand Down
8 changes: 4 additions & 4 deletions include/envoy/stats/stat_data_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ class StatDataAllocator {
* @return CounterSharedPtr a counter, or nullptr if allocation failed, in which case
* tag_extracted_name and tags are not moved.
*/
virtual CounterSharedPtr makeCounter(absl::string_view name, std::string&& tag_extracted_name,
std::vector<Tag>&& tags) PURE;
virtual CounterSharedPtr makeCounter(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags) PURE;
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @param name the full name of the stat.
Expand All @@ -45,8 +45,8 @@ class StatDataAllocator {
* @return GaugeSharedPtr a gauge, or nullptr if allocation failed, in which case
* tag_extracted_name and tags are not moved.
*/
virtual GaugeSharedPtr makeGauge(absl::string_view name, std::string&& tag_extracted_name,
std::vector<Tag>&& tags) PURE;
virtual GaugeSharedPtr makeGauge(StatName name, absl::string_view tag_extracted_name,
const std::vector<Tag>& tags) PURE;

/**
* Determines whether this stats allocator requires bounded stat-name size.
Expand Down
35 changes: 22 additions & 13 deletions include/envoy/stats/stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
#include <vector>

#include "envoy/common/pure.h"
#include "envoy/stats/symbol_table.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Stats {

class StatDataAllocator;
struct Tag;

/**
Expand All @@ -32,32 +34,39 @@ class Metric {
virtual std::string name() const PURE;

/**
* Returns the full name of the Metric as a nul-terminated string. The
* intention is use this as a hash-map key, so that the stat name storage
* is not duplicated in every map. You cannot use name() above for this,
* as it returns a std::string by value, as not all stat implementations
* contain the name as a std::string.
*
* Note that in the future, the plan is to replace this method with one that
* returns a reference to a symbolized representation of the elaborated string
* (see source/common/stats/symbol_table_impl.h).
* Returns the full name of the Metric as an encoded array of symbols.
*/
virtual const char* nameCStr() const PURE;
virtual StatName statName() const PURE;

/**
* Returns a vector of configurable tags to identify this Metric.
*/
virtual const std::vector<Tag>& tags() const PURE;
virtual std::vector<Tag> tags() const PURE;

/**
* Returns the name of the Metric with the portions designated as tags removed
* as a string. For example, The stat name "vhost.foo.vcluster.bar.c1" would
* have "foo" extracted as the value of tag "vhost" and "bar" extracted as the
* value of tag "vcluster". Thus the tagExtractedName is simply
* "vhost.vcluster.c1".
*
* @return The stat name with all tag values extracted.
*/
virtual std::string tagExtractedName() const PURE;

/**
* Returns the name of the Metric with the portions designated as tags removed.
* Returns the name of the Metric with the portions designated as tags
* removed as a StatName
*/
virtual const std::string& tagExtractedName() const PURE;
virtual StatName tagExtractedStatName() const PURE;

/**
* Indicates whether this metric has been updated since the server was started.
*/
virtual bool used() const PURE;

virtual SymbolTable& symbolTable() PURE;
virtual const SymbolTable& symbolTable() const PURE;
};

/**
Expand Down
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(const std::string& stat_name, std::vector<Tag>& tags,
virtual bool extractTag(absl::string_view stat_name, std::vector<Tag>& tags,
IntervalSet<size_t>& remove_characters) const PURE;

/**
Expand Down
14 changes: 11 additions & 3 deletions include/envoy/stats/tag_producer.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include "envoy/common/pure.h"
#include "envoy/stats/tag.h"

#include "absl/strings/string_view.h"

namespace Envoy {
namespace Stats {

Expand All @@ -16,12 +18,18 @@ class TagProducer {

/**
* Take a metric name and a vector then add proper tags into the vector and
* return an extracted metric name.
* return an extracted metric name. The tags array will be populated with
* name/value pairs extracted from the full metric name, using the regular
* expressions in source/common/config/well_known_names.cc. For example, the
* stat name "vhost.foo.vcluster.bar.c1" would have "foo" extracted as the
* value of tag "vhost" and "bar" extracted as the value of tag
* "vcluster", so this will populate tags with {"vhost", "foo"} and
* {"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.
*/
virtual std::string produceTags(const std::string& metric_name,
std::vector<Tag>& tags) const PURE;
virtual std::string produceTags(absl::string_view metric_name, std::vector<Tag>& tags) const PURE;
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
};

typedef std::unique_ptr<const TagProducer> TagProducerPtr;
Expand Down
1 change: 1 addition & 0 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ envoy_cc_library(
"//include/envoy/stats:stats_interface",
"//source/common/common:enum_to_int",
"//source/common/common:utility_lib",
"//source/common/stats:symbol_table_lib",
"@envoy_api//envoy/type:http_status_cc",
],
)
Expand Down
1 change: 1 addition & 0 deletions source/common/stats/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ envoy_cc_library(

envoy_cc_library(
name = "metric_impl_lib",
srcs = ["metric_impl.cc"],
hdrs = ["metric_impl.h"],
deps = [
":symbol_table_lib",
Expand Down
52 changes: 29 additions & 23 deletions source/common/stats/heap_stat_data.cc
Original file line number Diff line number Diff line change
@@ -1,35 +1,43 @@
#include "common/stats/heap_stat_data.h"

#include "common/common/lock_guard.h"
#include "common/common/logger.h"
#include "common/common/thread.h"
#include "common/common/utility.h"

namespace Envoy {
namespace Stats {

HeapStatData::HeapStatData(absl::string_view key) {
StringUtil::strlcpy(name_, key.data(), key.size() + 1);
HeapStatDataAllocator::~HeapStatDataAllocator() { ASSERT(stats_.empty()); }

HeapStatData* HeapStatData::alloc(StatName stat_name, SymbolTable& symbol_table) {
void* memory = ::malloc(sizeof(HeapStatData) + stat_name.size());
ASSERT(memory);
symbol_table.incRefCount(stat_name);
return new (memory) HeapStatData(stat_name);
jmarantz marked this conversation as resolved.
Show resolved Hide resolved
}

HeapStatDataAllocator::~HeapStatDataAllocator() { ASSERT(stats_.empty()); }
void HeapStatData::free(SymbolTable& symbol_table) {
symbol_table.free(statName());
this->~HeapStatData();
::free(this); // matches malloc() call above.
}

HeapStatData* HeapStatDataAllocator::alloc(absl::string_view name) {
// Any expected truncation of name is done at the callsite. No truncation is
// required to use this allocator. Note that data must be freed by calling
// its free() method, and not by destruction, thus the more complex use of
// unique_ptr.
std::unique_ptr<HeapStatData, std::function<void(HeapStatData * d)>> data(
HeapStatData::alloc(name), [](HeapStatData* d) { d->free(); });
HeapStatData& HeapStatDataAllocator::alloc(StatName name) {
using HeapStatDataFreeFn = std::function<void(HeapStatData * d)>;
std::unique_ptr<HeapStatData, HeapStatDataFreeFn> data_ptr(
HeapStatData::alloc(name, symbolTable()),
[this](HeapStatData* d) { d->free(symbolTable()); });
Thread::ReleasableLockGuard lock(mutex_);
auto ret = stats_.insert(data.get());
auto ret = stats_.insert(data_ptr.get());
HeapStatData* existing_data = *ret.first;
lock.release();

if (ret.second) {
return data.release();
return *data_ptr.release();
}
++existing_data->ref_count_;
return existing_data;
return *existing_data;
}

void HeapStatDataAllocator::free(HeapStatData& data) {
Expand All @@ -44,19 +52,17 @@ void HeapStatDataAllocator::free(HeapStatData& data) {
ASSERT(key_removed == 1);
}

data.free();
data.free(symbolTable());
}

HeapStatData* HeapStatData::alloc(absl::string_view name) {
void* memory = ::malloc(sizeof(HeapStatData) + name.size() + 1);
ASSERT(memory);
return new (memory) HeapStatData(name);
}

void HeapStatData::free() {
this->~HeapStatData();
::free(this); // matches malloc() call above.
#ifndef ENVOY_CONFIG_COVERAGE
void HeapStatDataAllocator::debugPrint() {
Thread::LockGuard lock(mutex_);
for (HeapStatData* heap_stat_data : stats_) {
ENVOY_LOG_MISC(info, "{}", symbolTable().toString(heap_stat_data->statName()));
}
}
#endif

template class StatDataAllocatorImpl<HeapStatData>;

Expand Down
Loading