support text format #12
Conversation
* add Encoder * add TextEncoder * add HttpHandler
| } | ||
| } | ||
|
|
||
| fn scarp(registry: &Registry, writer: &mut Write, encoder: &Encoder) -> Result<usize> { |
There was a problem hiding this comment.
scrap is frequently used in doc
There was a problem hiding this comment.
I guess this was supposed to be scrap.
There was a problem hiding this comment.
Oops, my poor english.
| fn on_response_writable(&mut self, encoder: &mut HyperEncoder<HttpStream>) -> Next { | ||
| match encoder.try_write(&self.buffer.0[self.write_pos..]) { | ||
| Ok(Some(n)) => { | ||
| if n == self.buffer.0.len() { |
There was a problem hiding this comment.
A bug here, fix later.
| colloctors_by_id: HashMap::new(), | ||
| dim_hashes_by_name: HashMap::new(), | ||
| }; | ||
| pub fn scrap_registry(registry: &Registry, writer: &mut Write, encoder: &Encoder) -> Result<usize> { |
There was a problem hiding this comment.
why using scrap here? confuse me.
| @@ -8,8 +8,12 @@ default = [] | |||
| dev = ["clippy"] | |||
|
|
|||
| [[bin]] | |||
| name = "example" | |||
| path = "example/simple_example.rs" | |||
| name = "example_hyper" | |||
There was a problem hiding this comment.
please follow http://doc.crates.io/guide.html#project-layout, maybe we don't need bin for examples.
|
|
||
| let c2 = counter.clone(); | ||
| thread::spawn(move || { | ||
| loop { |
There was a problem hiding this comment.
don't loop forever, some counters is ok enough.
There was a problem hiding this comment.
It does not create new clones, just increases count.
There was a problem hiding this comment.
what I mean is don't run this forever, run some times and then terminate the program.
| thread::spawn(move || { | ||
| for _ in 0..10 { | ||
| thread::sleep(Duration::from_millis(500)); | ||
| c2.inc_by(3.14159265358979323846264338327).unwrap(); |
There was a problem hiding this comment.
Why not use std::f64::consts::PI?
There was a problem hiding this comment.
I think we should use simple value like 1.0, 2.0, 3.0 in counter example.
Do go client use PI in counter?
|
|
||
| // TODO: check type. | ||
| // TODO: check consistency. | ||
| for metric in mf.take_metric().into_iter() { |
There was a problem hiding this comment.
you can use extend instead.
There was a problem hiding this comment.
existent_metrics is a protobuf::RepeatedField, unfortunately it does not support extend.
|
|
||
| written += try!(writer.write(name.as_bytes())); | ||
|
|
||
| written += try!(label_pairs_to_text(mc.get_label(), |
There was a problem hiding this comment.
seem that here can only support text format, how to support protocbuf later?
There was a problem hiding this comment.
It does not, only for TextEncoder.
| /// perform checks on the content of the metric and label names, | ||
| /// i.e. invalid metric or label names will result in invalid text format | ||
| /// output. | ||
| fn encode(&self, &[MetricFamily], &mut Write) -> Result<usize>; |
There was a problem hiding this comment.
Do we have to return the written bytes here? Can it be figured out by the caller?
There was a problem hiding this comment.
No, it returns the number of written bytes. Why? Because Content-Length in HTTP.
It is difficult for out caller to compute precise number of written bytes.
There was a problem hiding this comment.
In this case, header should be written before body. Then a buffer is needed to cache the output. We can figure out the body length by the buffer length's diff.
There was a problem hiding this comment.
There was a problem hiding this comment.
both are ok, but may Result<()> is more commonly used.
Prom go client encoder doesn't return size too.
There was a problem hiding this comment.
If return Result<()>, then write_writer is not needed, we can just use write!. The latter doesn't need to create another temporary string.
There was a problem hiding this comment.
Indeed, but encoder in go client counts number internally.
Maybe I should change Result<usize> to Result<()>, but let TextEncoder counts number internally too.
There was a problem hiding this comment.
no need, don't add it now.
|
|
||
| let help = mf.get_help(); | ||
| if !help.is_empty() { | ||
| try!(writer.write_all(format!("# HELP {} {}\n", name, escape_string(help, false)) |
| pub struct HttpHandler<'a> { | ||
| registry: Registry, | ||
| encoder: &'a (Encoder + 'a), | ||
| buffer: Vec<u8>, |
There was a problem hiding this comment.
In practice, we should use VecDeque.
There was a problem hiding this comment.
this is only example, I think ok here.
|
rest LGTM |
|
LGTM |
|
Ref #8 |
#7 #9
PTAL @siddontang @BusyJay