-
Notifications
You must be signed in to change notification settings - Fork 259
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
Just for maintainers information #431
Comments
/cc @tigrannajaryan for visibility only :) nothing we can do now. |
After the discussion with @bryce-b the summary is:
If Swift maintainers have payed attention to this repo and the deprecation notice they would have avoided both their problems since they also broke when upgraded to v0.15.0 because old collectors were not able to read the new "fake" ID. I don't think that this is Swift maintainers mistake, but a bad coincidence that the generator does not generate deprecated fields, and that they could not get signaled that they were using a "fake" deprecated id. |
Also the issue with "Python" and "Go" were both related to us trying to be smarter and keeping the old name with the fake ID, since instead of causing a compilation error, users were able to build applications with older API/SDK and newer Protos, so they started to send by mistake the new "fake" id (code compiled, but not as expected) that we added and collector was not ready to receive that new "fake" id and translate it into the old id. This would have been avoidable (and SHOULD) if the maintainers of these languages would not expose the proto generated files as a separate artifact and allow their users to upgrade these independently. |
@bogdandrutu Perhaps reintroduction of the backcompat "smartness" in opentelemetry collector could be considered? (at least until proto clients 0.18 can be considered "old enough to not support"). We did exactly that in our fork: fluxninja@1b57bad, fluxninja/opentelemetry-collector@b82d435. |
@krdln please file a bug there. Added also a hack, see open-telemetry/opentelemetry-collector#6342 |
To prevent this a separate issue is filed to discuss this policy: open-telemetry/opentelemetry-specification#3420 |
The way to handle json breaking changes implemented in #362 caused lots of issues (mainly because existing code continue to compile and in some cases users code upgrade the proto independently of the SDK see go case, or in other cases maintainers did not do the transition because code was not marked as deprecated see swift case), the recommended way is "json_name" annotation (see https://developers.google.com/protocol-buffers/docs/proto3)
The text was updated successfully, but these errors were encountered: