Change tracing to use Resource.to_json()

[ocelotl]

Could you create an issue to change tracing to use Resource.to_json(), in a way that's not breaking?

Originally posted by @lzchen in #2722 (comment)

[jeremydvoss]

Hey @ocelotl I'm interested in this ticket. Could you explain a bit more? What do we want to use the new Resource.to_json() for or instead of?

[srikanthccv]

@jeremydvoss Currently resource in tracing is formatted using this private _format_attributes method. This issue is to update such references to use .to_json method.

[jeremydvoss]

The _format_attributes and resource.to_json methods handle the null case differently. The former would produce {} while the later would produce:

{
    "attributes": {},
    "schema_url": ""
}

Furthermore, the to_json method actually produces a string and not a dictionary as expected. But that can be changed. @srikanthccv Do we want to add blank resource.attribute and resource.schema_url keys when there are no values, or keep the current behavior?

[lzchen]

@jeremydvoss
For tracing, the current behavior must be kept or else it would be a breaking change, since tracing is already released as stable. Try to keep the behavior exactly the same but change the way we implement the formatting of spans to use to_json instead of _format_attributes like how we format metrics record and logs.

[jeremydvoss]

Since the logic of the to_json method and the ultimate output behavior needs to remain the same, the only option is to use the to_json method but remove the key-value pairs that are empty. It's not as clean as I might like. I'll create a PR and see what the community thinks.

[srikanthccv]

@lzchen the console span exporter is intended for debugging purposes and people shouldn't rely it for any other purposes. It is okay to reformat the spans. Am i misunderstanding something?

[lzchen]

@srikanthccv
That's fine with me but if it's within our stable package I'm assuming the implications should mean that there shouldn't be any breaking changes. Unless if console span exporter is ommitted from our stable api surface, I would think the above would hold, regardless of the purpose. I don't have a strong opinion on this, just want to make sure our message is clear for exactly what we are deeming as "no breaking changes ever".

[srikanthccv]

We are explicit about the usage of this component in our stable package.

opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py

Lines 416 to 421 in 9fbc93b

	"""Implementation of :class:`SpanExporter` that prints spans to the
	console.
	This class can be used for diagnostic purposes. It prints the exported
	spans to the console STDOUT.
	"""

I don't think we have any breaking changes to our stable package interfaces. I am not sure under what category this comes under (behavioural change?). While the intended usage is for debugging purpose, I have seen people workaround things for their need. We can never know what happens in the wild. Your comment made me rethink my position. One potential case is when people have some big programs that they instrumented and they can't use any exporter which requires n/w access, so instead they write it to a file and periodically collect and parse this file to upload the telemetry to backend. I don't mind keeping the current behaviour.

[jeremydvoss]

Merged #2784