Skip to content

IsolatedStoreImpl::histograms() always empty #6582

@mergeconflict

Description

@mergeconflict

IsolatedStoreImpl is used in a few spots, mostly in tests. One example of this is stats_store_ in ServerInstanceImplTest. I was trying to add to the Stats unit test there, which asserts that a few essential server stats are populated, and noticed that only counters and gauges are tested, not histograms:

EXPECT_NE(nullptr, TestUtility::findCounter(stats_store_, "server.watchdog_miss"));
EXPECT_EQ(2L, TestUtility::findGauge(stats_store_, "server.concurrency")->value());
EXPECT_EQ(3L, TestUtility::findGauge(stats_store_, "server.hot_restart_epoch")->value());

I wanted to test the presence of a new histogram, so the next thing I discovered is that TestUtility has no findHistogram. And when I implemented it, it always failed. Finally, digging deeper, I discovered:

std::vector<ParentHistogramSharedPtr> histograms() const override {
  return std::vector<ParentHistogramSharedPtr>{};
}

IsolatedStoreImpl always returns an empty set of histograms. This makes sense: we're not using thread-local storage and merging histograms across threads, so there is no ParentHistogram. Likewise, deliverHistogramToSinks is empty, and when we allocate histograms in the isolated store, we don't allocate a parent:

histograms_([this](const std::string& name) -> HistogramSharedPtr {
  return std::make_shared<HistogramImpl>(name, *this, std::string(name), std::vector<Tag>());
})

Still, it would be nice for IsolatedStoreImpl::histograms() to return something. Would it be possible to create a trivial IsolatedParentHistogramImpl that just references a single histogram? @jmarantz

Metadata

Metadata

Assignees

Labels

bugstalestalebot believes this issue/PR has not been touched recently

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions