Use tags in agent metrics

[eundoosong]

Signed-off-by: Eundoo Song eundoo.song@gmail.com

Which problem is this PR solving?

• Change path-based metrics to new format in agent.
• Resolves: Agent metrics are not tagged #850

Short description of the changes

• jaeger and zipkin are pulled into format tag.
• Gist : https://gist.github.com/eundoosong/8d3e06a259b60ce0ff346799dd3f7f22

jaeger_agent_tc_reporter_batch_size{format="jaeger"} 0
jaeger_agent_tc_reporter_batch_size{format="zipkin"} 0
jaeger_agent_tc_reporter_batches_failures{format="jaeger"} 0
jaeger_agent_tc_reporter_batches_failures{format="zipkin"} 0
jaeger_agent_tc_reporter_batches_submitted{format="jaeger"} 0
jaeger_agent_tc_reporter_batches_submitted{format="zipkin"} 0
jaeger_agent_tc_reporter_spans_failures{format="jaeger"} 0
jaeger_agent_tc_reporter_spans_failures{format="zipkin"} 0
jaeger_agent_tc_reporter_spans_submitted{format="jaeger"} 0
jaeger_agent_tc_reporter_spans_submitted{format="zipkin"} 0

[codecov]

Codecov Report

Merging #950 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #950   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         136    136           
  Lines        6322   6320    -2     
=====================================
- Hits         6322   6320    -2

Impacted Files	Coverage Δ
cmd/agent/app/reporter/tchannel/reporter.go	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 933efb3...261f6f3. Read the comment docs.

[yurishkuro]

1. there is no need to add tc-reporter explicitly like that, since you can pass that string as the first argument to Namespace in L76 (see point 3 below).
2. since the metrics are changing anyway, maybe we should rename them to use underscores, as we did with other backend metrics?
3. I think historically tc was used to refer to tcollector, which was the TChannel name of the collector service (now it's jaeger-collector). When we implement Refactor Agent configuration #927, there will be a different type of reporter, e.g. grpc-reporter. Also, in some cases we internally run the agent with two reporters (using OtherReporters configuration of the builder), I don't even know if they distinguish their metrics. So my suggestion, at minimum, to rename this to tchannel-reporter, to make it more obvious what we're talking about, but we also need to figure out how metrics will work after Refactor Agent configuration #927.

cc @black-adder

[eundoosong]

2. since the metrics are changing anyway, maybe we should rename them to use underscores, as we did with other backend metrics?

I can change it using underscore like tchannel_reporter. Just curious, the reason I kept using - is normalizer in jaeger-lib already replaces them. Don't we need it?
https://github.com/jaegertracing/jaeger-lib/blob/master/metrics/prometheus/factory.go#L109

[yurishkuro]

actually I see that collector metrics are still using dots as separators, so never mind about understores

[eundoosong]

Ok, thanks!

we also need to figure out how metrics will work after #927.

I'll keep eyes on this.

[eundoosong]

• Move tc-reporter to Namespace as first argument.
• Rename to tchannel-reporter

[yurishkuro]

Thanks!