apache / apache/incubator-pegasus

Meta server crashed in prometheus-cpp library

Open
#1,965 1 comment 0 reactions 0 assignees View on GitHub
type/bug
Dominant language
C++
Stars
2.1k
Forks
328
PR merge metrics
No merged PRs in 30d

Description

Meta server crashed if creating a backup policy with a name `test_1-1` (contains `-` character), the assert information is as below:
```
meta_server: /data/jenkins/workspace/skv/prebuild_binary-multi-arch/sub-jobs/build-develop-x86_64/thirdparty/build/Source/prometheus-cpp/core/include/prometheus/family.h:142: prometheus::Family::Family(const string&, const string&, const std::map, std::basic_string >&) [with T = prometheus::Gauge; std::string = std::basic_string]: Assertion `CheckMetricName(name_)' failed.
```

The prometheus-cpp code:
```
bool CheckMetricName(const std::string& name) {
// see https://prometheus.io/docs/concepts/data_model/
auto reserved_for_internal_purposes = name.compare(0, 2, "__") == 0;
if (reserved_for_internal_purposes) return false;
#ifdef STD_REGEX_IS_BROKEN
return !name.empty();
#else
static const std::regex metric_name_regex("[a-zA-Z_:][a-zA-Z0-9_:]*");
return std::regex_match(name, metric_name_regex);
#endif
}

bool CheckLabelName(const std::string& name) {
// see https://prometheus.io/docs/concepts/data_model/
auto reserved_for_internal_purposes = name.compare(0, 2, "__") == 0;
if (reserved_for_internal_purposes) return false;
#ifdef STD_REGEX_IS_BROKEN
return !name.empty();
#else
static const std::regex label_name_regex("[a-zA-Z_][a-zA-Z0-9_]*");
return std::regex_match(name, label_name_regex);
#endif
}
```

So the metric name should:
- not prefixed by `__`
- not empty
- match `[a-zA-Z_][a-zA-Z0-9_]*`

See https://prometheus.io/docs/concepts/data_model/

> Metric names:
>
> - Metric names may contain ASCII letters, digits, underscores, and colons. It must match the regex [a-zA-Z_:][a-zA-Z0-9_:]*.
>
> Metric labels:
> - Labels may contain ASCII letters, numbers, as well as underscores. They must match the regex [a-zA-Z_][a-zA-Z0-9_]*.
> - Label names beginning with __ (two "_") are reserved for internal use.
> - Label values may contain any Unicode characters.
> - Labels with an empty label value are considered equivalent to labels that do not exist.

Contributor guide

Open the contributing guide

Research direction

Start by tracing backup-policy metric-name construction to the prometheus-cpp family.h assertion shown in the report. Reproduce the crash with the policy name `test_1-1`, then inspect the relevant metric validation path and existing tests, if present. Done means this input no longer aborts the meta server and the resulting metric name follows the stated Prometheus naming rules.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
observability-sre
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.