census-instrumentation / opencensus-go

A stats collection and distributed tracing framework
http://opencensus.io
Apache License 2.0
2.06k stars 327 forks source link

Enforce tag equivalence in `view.same()` #1279

Closed rubensf closed 2 years ago

rubensf commented 2 years ago

Otherwise, the lack of error may lead to developers wasting a lot of time trying to figure out why do their labels not show up.

dashpole commented 2 years ago

He @rubensf! Right now we are only accepting security fixes or critical bug fixes. Can you say more about the issue this fixes?

rubensf commented 2 years ago

Ah, I didn't notice this repo was being phased out. The problem here is that the check for two views being the same, in my semantic understanding, is incorrect - it currently returns that two views are the same even though they may have different tags.

This is an issue if you eg use zpages which registers some views at init(), or if you generally have a large enough codebase and register views in multiple spots. You can technically Unregister them and Register new ones, but if you're not very familiar with your codebase you may spend a few hours (as I did) trying to find out why your views with different tags are not showing up on your metrics... :)

That'd def be a behaviour change. I'm not sure if you'd count it as a critical bug fix though so feel free to close it.

dashpole commented 2 years ago

Thanks for your understanding. If others hit this issue we can reconsider it, but i'll close it for now. Hopefully your workaround will save them some time.