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

v1.23.0 added an abstract method to Meter, breaking subclasses #3710

Closed
alexmojaki opened this issue Feb 27, 2024 · 6 comments · Fixed by #3817
Closed

v1.23.0 added an abstract method to Meter, breaking subclasses #3710

alexmojaki opened this issue Feb 27, 2024 · 6 comments · Fixed by #3817
Labels
bug Something isn't working priority:p1 Issues that should be resolved in the upcoming release (except for zero-day hotfix release)

Comments

@alexmojaki
Copy link

alexmojaki commented Feb 27, 2024

We're developing an SDK that uses OpenTelemetry. This morning a user reported that after installing the SDK, everything they tried led to this error:

TypeError: Can't instantiate abstract class _ProxyMeter with abstract method create_gauge

_ProxyMeter here is our own subclass of Meter.

The changelog says:

this project adheres to Semantic Versioning.

I think releasing this change violates semver, as it was a minor version release that created errors in previously compliant code.

If this is intentional because users aren't supposed to subclass Meter themselves, please let me know so that we can constrain the dependency to <1.24 to avoid any future surprises. We have subclasses of Span, Tracer, TracerProvider, MeterProvider, Meter, and probably others, so from our perspective the abstract base classes need to be stable.

If this wasn't intentional, then in future I think you could either:

  1. Release such changes in a major version, or
  2. Add new 'abstract' methods to abstract base classes as normal methods (i.e. not decorated with @abstractmethod) but that simply raise NotImplementedError. That way subclasses can still be instantiated.

We've resolved the problem in our own library, but it's likely that there are other libraries currently giving new users the same error. That could be mitigated here by either:

  1. Releasing 2.0.0 with the same code, then yanking 1.23.0 from PyPI. This means that pip install opentelemetry-sdk<2 will install 1.22.0, while pip install opentelemetry-sdk==1.23.0 will still work.
  2. Releasing 1.23.1 with a backwards compatible fix such as (2) above.
@alexmojaki alexmojaki added the bug Something isn't working label Feb 27, 2024
@jha-hitesh
Copy link

jha-hitesh commented Feb 28, 2024

this issue is happening with us as well from yesterday, got below stacktrace while initialising opentelemetry metrics. we have also developed our own sdk on top of otel.

otel_srvr |     _set_meter_provider(meter_provider, log=True)
otel_srvr |   File "/.venv/lib/python3.8/site-packages/opentelemetry/metrics/_internal/__init__.py", line 799, in _set_meter_provider
otel_srvr |     did_set = _METER_PROVIDER_SET_ONCE.do_once(set_mp)
otel_srvr |   File "/.venv/lib/python3.8/site-packages/opentelemetry/util/_once.py", line 44, in do_once
otel_srvr |     func()
otel_srvr |   File "/.venv/lib/python3.8/site-packages/opentelemetry/metrics/_internal/__init__.py", line 797, in set_mp
otel_srvr |     _PROXY_METER_PROVIDER.on_set_meter_provider(meter_provider)
otel_srvr |   File "/.venv/lib/python3.8/site-packages/opentelemetry/metrics/_internal/__init__.py", line 173, in on_set_meter_provider
otel_srvr |     meter.on_set_meter_provider(meter_provider)
otel_srvr |   File "/.venv/lib/python3.8/site-packages/opentelemetry/metrics/_internal/__init__.py", line 463, in on_set_meter_provider
otel_srvr |     real_meter = meter_provider.get_meter(
otel_srvr |   File "/.venv/lib/python3.8/site-packages/opentelemetry/sdk/metrics/_internal/__init__.py", line 488, in get_meter
otel_srvr |     self._meters[info] = Meter(
otel_srvr | TypeError: Can't instantiate abstract class Meter with abstract methods create_gauge```

@freemansoft
Copy link

Are you sure this isn't a 3rd party interaction with the new code?

Wrote a small program that uses OTel to push network performance to a dashboard. The previous version used OpenCensus. Last week I installed it on a Mac and on a Windows machine with zero issues. Today I installed it on a Raspberry Pi and it broke with the same last line as your logs.

@freemansoft
Copy link

freemansoft commented Mar 4, 2024

This is almost certainly unrelated but...

Raspberry Pi which had this symptom was having trouble with dependency resolution.

I was on pip 20.1. Upgraded to pip 24.0. Re-fetched my project dependencies. with pip3 install -r requirements.txt Now my program runs fine.

@srikanthccv
Copy link
Member

This wasn't intentional (we added the method add_link which wasn't abstract although all the existing methods are abstract and simply warn no-op, to honour the semver guarantees #3618). However, this was overlooked during the create_gauge feature. We should have some tests/tooling to catch this kind of issue.

@aabmass
Copy link
Member

aabmass commented Mar 21, 2024

This was definitely a mistake

2. Releasing 1.23.1 with a backwards compatible fix such as (2) above.

Can someone send a PR and we can try to make a patch release ASAP?

@aabmass aabmass added the priority:p1 Issues that should be resolved in the upcoming release (except for zero-day hotfix release) label Mar 21, 2024
@lzchen
Copy link
Contributor

lzchen commented Mar 28, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p1 Issues that should be resolved in the upcoming release (except for zero-day hotfix release)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants