search

mozilla-services / common-rs

Common utilities for Mozilla server side apps
Mozilla Public License 2.0
3 stars 9 forks source link

feat(tracing-actix-web-mozlog): Add tools to require MozLog event types #26

Closed mythmon closed 2 years ago

mythmon commented 2 years ago
mythmon commented 2 years ago

I'm going to do some work using this PR in Merino to make sure it is actually a useful way to structure things before this is ready to merge.

mythmon commented 2 years ago

After thinking about this more, this approach means that the require-type-field tools only work if the output is the MozLog JSON. That means that we would only see this in prod, not in testing or development. This method isn't going to work.

Dexterp37 commented 2 years ago

After thinking about this more, this approach means that the require-type-field tools only work if the output is the MozLog JSON. That means that we would only see this in prod, not in testing or development. This method isn't going to work.

@mythmon does this mean the whole strategy we discussed about using "types" to route logs needs to be re-evaluated?

mythmon commented 2 years ago

No. We can still use types for routing, and we are still required to include types by Mozlog. I was a bit terse in my explanation above. My goal in this PR was to add tools that make it easier in development to find where types were forgotten and easier in all cases to provide types for libraries that use things like the standard log facade (which is unstructured and so cannot directly provide types).

My initial implementation here only included those tools when the log output was JSON, which does not help development. It tied semantic-level concerns to formatting concerns in a way that isn't good. Instead I'll need to introduce a new Tracing layer that can handle the semantic concerns independent of formatting, which is probably a better API anyways.

None of this stops us from manually auditing our logs, though it might make consuming logs from dependencies harder. I don't think we plan to do that much anyways though.