Constants for Erlang macros like Kind and Status in Elixir lib about opentelemetry-erlang HOT 9 CLOSED

Yeah, module attributes are just constants for that module. I'm actually not really sure what went into using macros to define atoms in the Erlang side and why they're capitalized, but just using atoms directly and having them be lower case would remove the weirdness of the Elixir usage. What was the reason for going that route?

from opentelemetry-erlang.

Since other BEAM based languages may not use erlang macro, and lowercase atom seems more "idiomatic" in Erlang... it would be better to define list of lowercase atoms with type definition. erlang library may use macro in it though..

from opentelemetry-erlang.

The reason it is :Error is because that is the Otel value. The user isn't supposed to see it so it not being idiomatic wasn't supposed to matter. The user should see only a constant, guarding against typos or if we ever need to change terms behind the scenes.

from opentelemetry-erlang.

@tsloughter Thanks for the comment. Here are my questions and suggestions:

The reason it is :Error is because that is the Otel value

What is "the Otel value"?

I found that those values (e.g. SERVER for SpanKind and Error for Status) are not even the exact values will be used in messages, since 1) OpenTelemetry supports both gRPC (protobuf - so integer value will be used) and HTTP (via JSON mapping, which uses the protobuf label name), so no single "value" there but this library's internal representation and 2) this library actually translates the internal values (OTEL_STATUS_ERROR) to OTLP value (ref) (e.g. STATUS_CODE_ERROR) as the spec says.

So why not using lowercase atoms (e.g. error) which are idiomatic? Similar to that most HTTP libraries use lowercase atoms for HTTP request (e.g. take post , not "POST", and use the corresponding HTTP method).

The user isn't supposed to see it so it not being idiomatic wasn't supposed to matter

I think it should be rather "users care the interface to this library while not caring what value this library will use in messages behind the scene."

As a dev, I prefer to have consistent, erlang/elixir idiomatic way to use libraries, whenever possible.

#155 is adding functions to return those "Otel values" that should be opaque to users - is it common in Erlang world? It's cumbersome (although possible) to pattern match for such dynamic value. Since they're enum, as far as we pick the easy-to-use value for the representation (e.g. lowercase atom), and the enum are pretty static (per OpenTelemetry spec), it doesn't seem useful. (e.g. I'd prefer to use atom and use Dialyzer)

For flexibility and extensibility, this library may accept both atom (e.g. server) for known values (which will be translated) or strings (e.g. "SERVER") to support values added in the future, or to use the value as-is.

from opentelemetry-erlang.

@chulkilee by "otel value" I simply meant the quoted term used in https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-status and for Kind.

is it common in Erlang world?

No, in Erlang the macros are used. Those functions were only added as a way for the Elixir code to access those macro values.

Exposing functions is the only way Elixir offers to expose constant values, I'd have used another way if it was available.

If it is this big of a deal then we can switch to lowercase (tho I find that odd too since no matter what in Elixir you have to have the : prefix :) to be idiomatic in Elixir.

I think it should be rather "users care the interface to this library while not caring what value this library will use in messages behind the scene."

Na, it is more than that, as a user I want to be told at compile time if this is wrong. With macros in Erlang this is done if I mistype a Kind macro I'm told, not so if I put the atom sever instead of server -- it hopefully gets caught by dialyzer but not good to bet on.

But again, I'm fine with a patch to lowercase everything if that is what Elixir users expect instead of functions for constants.

from opentelemetry-erlang.

Thanks @tsloughter

For the value:

It seems like OpenTelemetry API specification (e.g. Tracing API) defines possible values without specific representation, and OpenTelemetry Protocol specification defines own ProtoBuf schema and JSON mapping from the schema for representation of the values in gRPC/HTTP protocoal.

Also Tracing API says:

While languages and platforms have different ways of representing data, this section defines some generic requirements for this API.

Therefore actual representation inside library doesn't matter. For example, python and java libraries use their canonical way to represent the status code values - not using the "Otel value" (e.g. Error).

So no need to use the "Otel value" in the API spec as-is. My suggestion is to use lowercase atom for all those values, since lowercase is more widely used in Erlang. Hopefully before hitting 1.0 😄

For how to get the value:

Na, it is more than that, as a user I want to be told at compile time if this is wrong.

Oh I also want that 😉 but I'm against requiring calling functions to get the enum-like atom value defined by library. There is no need to "hide" them unless the library wants to change the representation freely without users' changes. Also note that I can even dynamically call functions (e.g. apply) so there is no 100% guranteed compile-time check if a user is not using it as intended..

But yeah I really hope I have enum-like value for constant with compile-time check... but that's not just available in Elixir out-of-box. Using macro is one option, but it has same limitation with calling functions 😢

from opentelemetry-erlang.

Right, it isn't a requirement, I've just found it simpler to default to what the API says to get around discussion/disagreement about naming. Obviously a number of things differ, we don't use EndSpan but instead end_span. So changing to lowercase is fine.

from opentelemetry-erlang.

But again, the values aren't supposed to be used, so lowercase being how Erlang does atoms doesn't matter :) No one in Erlang would be writing 'Error'.

from opentelemetry-erlang.

I think #175 resolves this issue.

from opentelemetry-erlang.