Add basic opencensus metrics to message handlers #297
Conversation
ghost
assigned 
|
I would hook the messages just once, and use the Type String as the label. I think the views should be returned directly from the IpfsDHT type, MetricViews() or something. Store them together in a struct on the IpfsDHT type. There's got to be a better default way to handle the distributions (prometheus infers them automatically). I think this change should expose publicly at most one new type, a struct that aggregates the views. |
|
Needs support for gx. |
|
Needs outbound message counting (much more interesting). |
|
@vyzo LOC or #commits? |
|
CI failing because we don't have 0.19.2 in gx. |
anacrolix
force-pushed
the
feat/oc-metrics
branch
from
ee043eb
to
dd49f0e
Compare
anacrolix
force-pushed
the
feat/oc-metrics
branch
from
dd49f0e
to
e4666c5
Compare
Experimentation elsewhere has found it not to be useful in the way I expected.
|
I've rebased and removed message write latencies, as it was a rabbit hole. |
|
@lanzafame @frrist @vyzo @bigs I'd like to merge this, and add support for error counts etc. in future PRs. Please review. |
| return tag.Upsert(KeyMessageType, m.Type.String()) | ||
| } | ||
|
|
||
| const namePrefix = "libp2p_dht_kad_" |
There was a problem hiding this comment.
Incorrect format of name prefix. OC converts to the appropriate format for the configured backend.
| return tag.Upsert(KeyMessageType, m.Type.String()) | ||
| } | ||
|
|
||
| const namePrefix = "libp2p_dht_kad_" |
There was a problem hiding this comment.
Variable not required, as this won't change and if it ever does, find-replace will be perfectly acceptable.
| KeyLocalPeerID.Name(), KeyInstanceID.Name()) | ||
| ) | ||
|
|
||
| var Views = []*view.View{{ |
There was a problem hiding this comment.
Move views to individual views, that are exported. This way they will appear in godocs and users will be able to register the ones that they want by name, instead of either always having to register all of them, register them by index (potentially getting the wrong metrics if the order of the slice is ever changed), or copying them over to their own code just so they can make the choice.
| dht.tagMutators()..., | ||
| ) | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
Don't panic in library code.
|
|
||
| stats.RecordWithTags( | ||
| ctx, | ||
| []tag.Mutator{ |
There was a problem hiding this comment.
Use UpsertMessageType function
There was a problem hiding this comment.
Better yet, move upserting of the message type to a tag.New so that the ctx has the message type and then remove the upsert of message type from the latency record statement at the end of the function.
|
I'll cannabalize anything here that was missed in #317. |
No description provided.