search

open-telemetry / opentelemetry-cpp

The OpenTelemetry C++ Client
https://opentelemetry.io/
Apache License 2.0
890 stars 426 forks source link

Elasticsearch Log exporter "resources" are under "resources" key while it should be under the "root" #3119

Closed ShadowMaxLeb closed 2 weeks ago

ShadowMaxLeb commented 3 weeks ago

Not exactly a bug but this is to continue the work I have done here https://github.com/open-telemetry/opentelemetry-cpp/issues/3091

It is open to discussion but I think, like for attributes that resources should also be by default in the root object and that it would be up to the user to add different levels if necessary.

This would make sense as for instance in the resources we would put user.id which is common to all logs and that would not make sense to "manually" add it through the log attributes every time we call EmitLogRecord

ShadowMaxLeb commented 2 weeks ago

@marcalff any thoughts on this ? Should I make a PR so we can discuss it ?

marcalff commented 2 weeks ago

@marcalff any thoughts on this ? Should I make a PR so we can discuss it ?

@ShadowMaxLeb,

Sorry for the delay.

Not familiar with the data format to use with the elastic search exporter, so hard to tell what exact format the exporter should produce.

Looking at the ECS doc https://www.elastic.co/guide/en/ecs/8.11/ecs-user.html it says, in the field reuse section:

Note also that the user fields may be used directly at the root of the events.

So, this is the issue reported here (don't put fields under root/resources) if I understand correctly.

Makes sense to me.

ShadowMaxLeb commented 2 weeks ago

Yes this is exactly it.

If there are resources (common to all logs) we have to let the user put them at the root.

Since nlohmann::json allows to do json["client.id"] that will end up being { "client": { "id" : "xxxx" } } this will let all users do as they like.

The only thing is that if anyone was using the exporter it will break how they were receiving logs.