Skip to content
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

Enum attributes with allow_custom_values not set defaults to true #218

Closed
bryannaegele opened this issue Jun 23, 2024 · 4 comments
Closed

Comments

@bryannaegele
Copy link
Contributor

bryannaegele commented Jun 23, 2024

Describe the bug

I'm running into an issue where enum attributes are not lining up with what is in the spec. Every enum attribute in the registry (rpc for example) has "allow_custom_values": true present. system is the only attribute that allows custom values. No other enums in that group explicitly set the value. Updating the file with "allow_custom_values": false for the remaining enums produces the expected output

image

To Reproduce

Steps to reproduce the behavior:

  1. Run the generator with v1.26.0.
  2. All enum attributes without allow_custom_values set in the model will have it added with a default value of true.
  3. Add allow_custom_values for the remaining enums to false in rpc.yaml.
  4. Run the generator and output is as expected.

Expected behavior

Enum attributes without allow_custom_values explicitly set in the model should default to false.

Additional context
Add any other context about the problem here.

@lquerel
Copy link
Contributor

lquerel commented Jun 23, 2024

Adding @jsuereth and @lmolkova to this issue as I'm not sure of the final decision regarding allow_custom_values

If I recall correctly, in March this year, the Semantic Conventions SIG decided to remove this field entirely. I believe the reasoning was that with allow_custom_values set to false, schema evolution was not feasible. Liudmila or Josh, please confirm.

@bryannaegele
Copy link
Contributor Author

That would make sense. The specific issue we have is since we don't have enums in Erlang or Elixir, we treat these only as types for guidance, whereas a standard attribute has both a function returning the value and a type signature. When custom values are allowed, we append | atom() so tooling knows it is allowed.

If false needs to be removed that'd be totally fine, but we would need there to be no property present at all. We just need some way to determine custom values are explicitly allowed. I believe that is the way the previous builder handled it when we started on 1.25. If it wasn't there, no default was added.

@lmolkova
Copy link
Contributor

The allow_custom_values will be removed/deprecated, more context in the #520

The TL;DR - all enums are open, there are no closed enums.

In other words, if someone does not use auto-generated semconv, they can write setAttribute("enum.attribute.foo", "bar") (for string enum) and it will be valid. I.e. there is no reason for code generation to separate open and closed enums.

@bryannaegele
Copy link
Contributor Author

Gotcha. Thanks for the context @lmolkova. We'll adjust our approach to supporting them. 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants