-
Notifications
You must be signed in to change notification settings - Fork 709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Naming issues for span structured data. #1423
Comments
Just looking into this a bit more, serde_json will only consider the last of duplicate keys. This means that in order to correctly extract information here we have to either find an alternative json deserialize which supports duplicate keys, or roll a manual one. ouch! Would you take a pull request for |
Changing the JSON format is considered a breaking change. We could make a change like this now by making it an opt-in configuration option. My two cents would be to use a naming scheme where names are prefixed with |
Personally I would say less is more... People using structured data are using tracing, so can be told certain keys are reserved. I think right now what's missing is clear recommendation. For example I'm using tracing in a lib. What can I count on? Will subscribers print span data? I can't count on it. They have the freedom not to, and several do, which means right now I have found no way of having sensical logs on Wasm. And as a lib author I don't control what subscriber get's used even if there was one that worked on Wasm. Freedom is great, but I think for something like this convention/recommendation would be greater. And I think the tracing API is already very complicated.
If a conclusion would be reached, I'd be up for filing a PR. Would it be a config option on the subscriber, or a crate feature? Is there a milestone/date for breaking changes on tracing-subscriber? |
I would find it useful to be able to make arbitrary changes to the field names instead of having them hard-coded. In particular, an internal tool expects the level to be called |
That API could look like this: impl SubscriberBuilder<JsonFields, Format<Json>> {
fn with_renamed_field(original_name: &'static str, new_name: &'static str) -> Self;
} |
@jyn514 Thanks for sharing. I do think it is a bit orthogonal to this issue. They will still need to have a default name, and that default name should still be unique enough so it doesn't overlap with common choices by users. Also the need to pre-process the json before it is serialized could be generalized better with an API that gives you |
This is a great idea! That makes it much more flexible, something like this :) impl SubscriberBuilder<JsonFields, Format<Json>> {
fn inspect(record: &mut serde_json::Value) -> Self;
} |
Bug Report
Version
test_tracing v0.1.0 (/data/doc/code/test_tracing)
├── tracing v0.1.26
│ ├── tracing-attributes v0.1.15 (proc-macro)
│ └── tracing-core v0.1.18
└── tracing-subscriber v0.2.18
├── tracing v0.1.26 ()
├── tracing-core v0.1.18 ()
├── tracing-log v0.1.2
│ └── tracing-core v0.1.18 ()
└── tracing-serde v0.1.2
└── tracing-core v0.1.18 ()
Platform
Linux desktop.home 5.12.8-arch1-1 #1 SMP PREEMPT Fri, 28 May 2021 15:10:20 +0000 x86_64 GNU/Linux
Description
Some of the first field names for structured data where
type
andname
. I ran into some quirky issues with both, so I wanted to report those here and raise the question as to whether this is desired behavior.Since
type
is a reserved keyword, my first attempt wasr#type
.Produces:
without json:
NOTES:
my_span
is the span name andBobby
is an actual name field.r#
get's removed fromr#name
, but not fromr#type
???Using quotes
Json:
default fmt:
NOTES:
r#
, and it's what's suggested in the docs, fair play. It still has duplicate fields.I would like to suggest:
__span_name
instead of very common ones likename
.r#
from all keys the same.The text was updated successfully, but these errors were encountered: