search

hrxi / tracing-loki

A tracing layer for shipping logs to Grafana Loki
Apache License 2.0
43 stars 20 forks source link

Add span fields to events #2

Closed chrismanning closed 2 years ago

chrismanning commented 2 years ago

I noticed that the fields on spans weren't appearing in Loki when I could see them in the console output while testing. Had a look in the code and saw that the span fields were being accumulated but were not sent along with the rest of the event data.

hrxi commented 2 years ago

Could you instead add another struct field for the span fields with #[serde(flatten)]? This way, we can avoid some allocations.

chrismanning commented 2 years ago

Could you instead add another struct field for the span fields with #[serde(flatten)]? This way, we can avoid some allocations.

Good call. Done that now.

I did also have a question: what's the purpose of having both labels and extra_fields in the Layer? It was my understanding that you would define labels as application-level attributes, then define fields in spans or directly in events. What would you use the extra_fields in Layer for?

hrxi commented 2 years ago

Thanks!

I did also have a question: what's the purpose of having both labels and extra_fields in the Layer? It was my understanding that you would define labels as application-level attributes, then define fields in spans or directly in events. What would you use the extra_fields in Layer for?

The idea is that labels should only be used for attributes that have a small amount of values, according to the Loki documentation. For stuff like a randomly generated run ID or a CI run number, this is inadequate because they have an infinite amount of possible values.

hrxi commented 2 years ago

Do you need a new release with this commit?

chrismanning commented 2 years ago

Thanks!

I did also have a question: what's the purpose of having both labels and extra_fields in the Layer? It was my understanding that you would define labels as application-level attributes, then define fields in spans or directly in events. What would you use the extra_fields in Layer for?

The idea is that labels should only be used for attributes that have a small amount of values, according to the Loki documentation. For stuff like a randomly generated run ID or a CI run number, this is inadequate because they have an infinite amount of possible values.

Understood, thanks for the response.

Do you need a new release with this commit?

Yes please. It's not urgent though as I can point at the git repo directly in the mean time.

hrxi commented 2 years ago

Released a new version tracing-loki 0.2.0 on crates.io. Sorry for the delay, when I tried publishing it last I got sidetracked looking up how to write a proper changelog.