Skip to content

Commit

Permalink
desc: Replace regex name validation with functions
Browse files Browse the repository at this point in the history
This allows dropping the dependency on regex, making the instrumentation
library much more lightweight.

This also fixes a bug with metric name validation, where an initial
colon was incorrectly disallowed. See data model for correct format:
  https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels

This may be because the library closely follows the official Go library,
which also had this issue.

Refs tikv#85
Refs prometheus/client_golang#255
  • Loading branch information
kamalmarhubi committed Nov 6, 2016
1 parent d0b20d3 commit 2482dac
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 10 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ clippy = {version = "^0", optional = true}
fnv = "1.0.3"
lazy_static = "0.2.1"
libc = "0.2"
regex = "0.1"

[dependencies.hyper]
version = "0.9"
Expand Down
51 changes: 43 additions & 8 deletions src/desc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,54 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::ascii::AsciiExt;
use std::collections::{HashMap, BTreeSet};
use std::hash::Hasher;

use fnv::FnvHasher;
use regex::Regex;

use proto::LabelPair;
use errors::{Result, Error};
use metrics::SEPARATOR_BYTE;

lazy_static! {
static ref VALID_METRIC_NAME: Regex = Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_:]*$").unwrap();
static ref VALID_LABEL_NAME: Regex = Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").unwrap();
// Details of required format are at
// https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
fn is_valid_metric_name(name: &str) -> bool {
// Valid metric names must match regex [a-zA-Z_:][a-zA-Z0-9_:]*.
fn valid_start(c: char) -> bool {
c.is_ascii() && match c as u8 {
b'a'...b'z' | b'A'...b'Z' | b'_' | b':' => true,
_ => false,
}
}

fn valid_char(c: char) -> bool {
c.is_ascii() && match c as u8 {
b'a'...b'z' | b'A'...b'Z' | b'0'...b'9' | b'_' | b':' => true,
_ => false
}
}

name.starts_with(valid_start) && !name.contains(|c| !valid_char(c))
}

fn is_valid_label_name(name: &str) -> bool {
// Valid label names must match regex [a-zA-Z_][a-zA-Z0-9_]*.
fn valid_start(c: char) -> bool {
c.is_ascii() && match c as u8 {
b'a'...b'z' | b'A'...b'Z' | b'_' => true,
_ => false,
}
}

fn valid_char(c: char) -> bool {
c.is_ascii() && match c as u8 {
b'a'...b'z' | b'A'...b'Z' | b'0'...b'9' | b'_' => true,
_ => false
}
}

name.starts_with(valid_start) && !name.contains(|c| !valid_char(c))
}

/// Desc is the descriptor used by every Prometheus Metric. It is essentially
Expand Down Expand Up @@ -83,7 +118,7 @@ impl Desc {
return Err(Error::Msg("empty help string".into()));
}

if !VALID_METRIC_NAME.is_match(&desc.fq_name) {
if !is_valid_metric_name(&desc.fq_name) {
return Err(Error::Msg(format!("'{}' is not a valid metric name", desc.fq_name)));
}

Expand All @@ -93,7 +128,7 @@ impl Desc {
let mut label_names = BTreeSet::new();

for label_name in const_labels.keys() {
if !VALID_LABEL_NAME.is_match(label_name) {
if !is_valid_label_name(label_name) {
return Err(Error::Msg(format!("'{}' is not a valid label name", &label_name)));
}

Expand All @@ -111,7 +146,7 @@ impl Desc {
// cannot be in a regular label name. That prevents matching the label
// dimension with a different mix between preset and variable labels.
for label_name in &desc.variable_labels {
if !VALID_LABEL_NAME.is_match(label_name) {
if !is_valid_label_name(label_name) {
return Err(Error::Msg(format!("'{}' is not a valid label name", &label_name)));
}

Expand Down Expand Up @@ -198,7 +233,7 @@ mod tests {

#[test]
fn test_invalid_metric_name() {
for &name in &["-dash", "9gag", ":colon", "has space"] {
for &name in &["-dash", "9gag", "has space"] {
let res = Desc::new(name.into(), "help".into(), vec![], HashMap::new())
.err()
.expect(format!("expected error for {}", name).as_ref());
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ extern crate fnv;
extern crate lazy_static;
extern crate hyper;
extern crate libc;
extern crate regex;

mod errors;
mod encoder;
Expand Down

0 comments on commit 2482dac

Please sign in to comment.