opentelemetry-python icon indicating copy to clipboard operation
opentelemetry-python copied to clipboard

Add support for Protobuf 5

Open aabmass opened this issue 1 year ago • 5 comments

Protobuf 5 was released but it currently conflicts with opentelemetry-proto package https://github.com/open-telemetry/opentelemetry-python/blob/762bd8f2629a1babc0eee4361a4873433511e836/opentelemetry-proto/pyproject.toml#L27-L29

This is by design since protobuf generated code is supposed to match the runtime protobuf library version. When 4.x was released we had a lot of issues and I opened https://github.com/protocolbuffers/protobuf/issues/11123 to get some clarification. We know have some docs https://protobuf.dev/support/cross-version-runtime-guarantee/ which make it clear New Gencode + Old Runtime = Never Allowed. I.e. regenerating code with grpcio-tools 5 will break compatibility with protobuf 4.

This puts us in a tricky spot of choosing which major version to support. The good news is

Starting with the 2025Q1 release, the protobuf project will adopt a rolling compatibility window for major versions..

That doesn't help right now, but hopefully will for the next major version. Because they are planning to fix things, I'm hesitant on making our own solution like keeping two copies of generated code and dynamically choosing between them at import time.


Related

  • [ ] https://github.com/open-telemetry/opentelemetry-python/issues/3932#issuecomment-2146072966
  • [ ] https://github.com/open-telemetry/opentelemetry-python/pull/3931

aabmass avatar Jun 06 '24 17:06 aabmass

The simplest solution is to only support a single protobuf major version. Once the ecosystem has majority moved over to Protobuf 5, we can switch over to just supporting it. I don't have a good signal on when that is.

It would helpful if people can share what version of Protobuf you are using and if this is a critical blocker for other version upgrades.

aabmass avatar Jun 06 '24 17:06 aabmass

The new version of the authzed library already uses protobuf 5. And we are planning an update, but a dependency conflict is preventing us.

Hope for a speedy resolution

AlexCAPS avatar Jun 18 '24 14:06 AlexCAPS

This may be interesting: https://github.com/googleapis/python-api-common-protos/compare/v1.63.1...v1.63.2

xrmx avatar Jul 01 '24 07:07 xrmx

We know have some docs https://protobuf.dev/support/cross-version-runtime-guarantee/ which make it clear New Gencode + Old Runtime = Never Allowed. I.e. regenerating code with grpcio-tools 5 will break compatibility with protobuf 4.

Right, but can't the runtime be updated even if the generated files are old? e.g. the google cloud tools like pubsub allow anything from >=3.20.2 up to <6.0.0dev. So in this situation, if you continue to generate the files with grpcio-tools 4 and allow more modern runtimes, it should work shouldn't it?

https://github.com/googleapis/python-pubsub/blob/main/setup.py#L45

Upon investigation, it turns out that they are using proto-plus to define their messages like class Foo(proto.Message):....

ehiggs avatar Jul 23 '24 15:07 ehiggs

Right, but can't the runtime be updated even if the generated files are old? e.g. the google cloud tools like pubsub allow anything from >=3.20.2 up to <6.0.0dev. So in this situation, if you continue to generate the files with grpcio-tools 4 and allow more modern runtimes, it should work shouldn't it?

My understanding is that, until Q1 2025 when they launch rolling compatibility, it is not guaranteed to work:

Protobuf cross-version usages outside the guarantees are error-prone and not supported. Version skews can lead to flakes and undefined behaviors that are hard to diagnose, even if it can often seem to work as long as nothing has changed in a source-incompatible way.

It is only guaranteed to work across the same minor version:

Within a single major runtime version, generated code from an older version of protoc will run on a newer runtime.

aabmass avatar Jul 23 '24 16:07 aabmass

We are using protobuf 5, and just started using OpenTelemetry. Our current workaround is to have our own version of opentelemetry-python that contains protobuf 5 generated files. Not ideal, but without that OpenTelemetry would be a non-starter for us for the time being.

Yesterday though I encountered an interesting approach to this problem that might be a good solution for OpenTelemetry too. wandb generates pb2 files for all protobuf versions, then dynamically imports the appropriate one based on the protobuf runtime version, as shown in the link below:

https://github.com/wandb/wandb/blob/7c8ead46482d3beafaeb8ba9f579e056b3812a68/wandb/proto/wandb_base_pb2.py

chrismiller avatar Sep 13 '24 09:09 chrismiller

Thanks for the data point. It's been a few months now, I think we should just upgrade to protobuf 5. Let's try to get it in for the next release.

Yesterday though I encountered an interesting approach to this problem that might be a good solution for OpenTelemetry too. wandb generates pb2 files for all protobuf versions, then dynamically imports the appropriate one based on the protobuf runtime version, as shown in the link below:

We have considered that as well, but since protobuf says they'll have rolling support in the next 6 months, I think we could just wait. Hopefully the next major version will be covered.

aabmass avatar Sep 20 '24 18:09 aabmass

We have considered that as well, but since protobuf says they'll have rolling support in the next 6 months, I think we could just wait. Hopefully the next major version will be covered.

Does this mean that otel would only upgrade when the next major version of the protobuf runtime is released? Wouldn't this mean anyone using opentelemetry would not be able to upgrade their appilcations who are using protobuf for other purposes until that time? Seems not great to limit application upgrades because of telemetry.

I dug more into how GCP client libraries work and they use proto-plus to generate the protobuf. BUT they recently added opentelemetry as an optional dependency so now googles' own client libraries would now depend on opentelemetry's reliance on an old version of protobuf. :dizzy_face:

If wandb is not an option, maybe proto-plus would be useful to side step the versioning issue.

ehiggs avatar Sep 22 '24 09:09 ehiggs

We have considered that as well, but since protobuf says they'll have rolling support in the next 6 months, I think we could just wait. Hopefully the next major version will be covered.

We had been sitting on this for quite a while hoping it would resolve itself, but we have some partners that have to move forward. 6 months out, is a very long time to wait, we will for sure be forced to drop our telemetry.

krpatter-intc avatar Sep 23 '24 00:09 krpatter-intc

We had been sitting on this for quite a while hoping it would resolve itself,

Apologies, I was off a bit my re-collection was off a version and it was the 4.x issue...so indeed its just a yearly issue I guess we will have here until protobuf supports. We just may lose this round and have to back out telemetry until it resolved.

krpatter-intc avatar Sep 23 '24 01:09 krpatter-intc

@ehiggs

Does this mean that otel would only upgrade when the next major version of the protobuf runtime is released?

No, I'm trying to say let's fix this specific issue for the next release, by upgrading to protobuf 5 and dropping support for protobuf 4.

This should be the last time that a new protobuf major version causes any compatibility issue. We don't need a bespoke fix as they are working on rolling support for generated code across major versions.

Does that sound good to you?

aabmass avatar Sep 24 '24 16:09 aabmass

~A data point on usage before we move forward. I wasn't able to find PyPI stats that break down by package version.~

I was able to make the breakdown of PyPI downloads of protobuf per major version with the PyPI BigQuery dataset. It looks like protobuf v5 has just surpassed v4 in downloads in this past couple of weeks.

SQL

#standardSQL
SELECT
  COUNT(*) AS num_downloads,
  REGEXP_EXTRACT(file.version, r'(\d+)\.\d+\.\d+') AS major_version,
  DATE(timestamp) AS timestamp_day
FROM
  `bigquery-public-data.pypi.file_downloads`
WHERE
  file.project = 'protobuf'
  -- Only query the last 60 days of history
  AND DATE(timestamp) BETWEEN DATE_SUB(CURRENT_DATE(), INTERVAL 60 DAY)
  AND CURRENT_DATE()
GROUP BY
  major_version,
  timestamp_day

image

aabmass avatar Sep 27 '24 14:09 aabmass

Closing as support for protobuf5 was merged 🚀 #4206

emdneto avatar Oct 14 '24 19:10 emdneto

Thanks for merging support! Is there an issue I can follow to find out when this would be released?

hauntsaninja avatar Oct 18 '24 20:10 hauntsaninja

@ocelotl @lzchen I see the two of have cut all of the recent releases. Do you know if there is one planned in the near future? It would be really great to see protobuf 5 support released.

mortenmj avatar Oct 24 '24 20:10 mortenmj

@hauntsaninja @mortenmj We don't have a fixed release date, but we do our best to have a monthly cadence. We discussed it's time to 1.28.0 at today's SIG, so the next version will probably be released soon

emdneto avatar Oct 24 '24 20:10 emdneto

protobuf 5 support is a critical blocker for us; hope the new release can come out soon. Thanks @emdneto!

omraj21 avatar Oct 25 '24 19:10 omraj21

Same as our project

cnjack avatar Oct 29 '24 11:10 cnjack

Same for ours

speksen-kb01 avatar Oct 31 '24 06:10 speksen-kb01

Are there any updates on a release date? This is a critical blocker for us.

AtoZdevelopment avatar Nov 04 '24 09:11 AtoZdevelopment