chore: generate warnings for events that will be removed in Juju 4.0

[tonyandrewmeyer]

A DeprecationWarning is emitted whenever an instance of CollectMetricsEvent, PreSeriesUpgradeEvent, or PostSeriesUpgradeEvent is created (which should only be whenever there is an observed event of that type).

Documentation warnings were already added in an earlier PR.

Fixes #1280

[benhoyt]

Looks good to me, thanks. Just one comment asking where the error (stacklevel) points.

[benhoyt]

Where does stacklevel=2 point? Can you give some example output?

[tonyandrewmeyer]

With 2, it's in the framework, for example:

====================================================================== warnings summary =======================================================================
tests/unit/test_charm.py::TestCharm::test_metrics
  /home/tameyer/scratch/collectmetrics/.tox/unit/lib/python3.11/site-packages/ops/framework.py:345: DeprecationWarning: collect-metrics events will not be emitted from Juju 4.0 onwards - consider using the Canonical Observability Stack
    event = self.event_type(Handle(self.emitter, self.event_kind, key), *args, **kwargs)

With 1, it would be in the __init__, which seemed to low-level.

Maybe it should be all the way outside of ops for tests? That would be 3, for example:

====================================================================== warnings summary =======================================================================
tests/unit/test_charm.py::TestCharm::test_metrics
  /home/tameyer/scratch/collectmetrics/tests/unit/test_charm.py:27: DeprecationWarning: collect-metrics events will not be emitted from Juju 4.0 onwards - consider using the Canonical Observability Stack
    self.harness.charm.on.collect_metrics.emit()

I do think that these are most likely to be seen in tests, since DeprecationWarnings are ignored by default so these would only show up in Juju's debug-log if a charm had changed the default warnings filter, and that seems unlikely.

A 3 outside of tests, if the filter was changed, would look like something like this in debug-log:

/var/lib/juju/agents/unit-collectmetrics-0/charm/venv/ops/_main.py:134: DeprecationWarning: collect-metrics events will not be emitted from Juju 4.0 onwards - consider using the Canonical Observability Stack
  event_to_emit.emit(*args, **kwargs)

I wasn't able to get juju collect-metrics to work (I get failed to collect metrics: could not read stdout, even with main) so did rely on using jhack fire to get this.

An alternative would be to put this in the observe method instead, and then a stacklevel=2 would be to the observe call in the charm. That would look like this in debug-log (and would similarly change in the test output):

unit-collectmetrics-0: 17:44:14 WARNING unit.collectmetrics/0.update-status /var/lib/juju/agents/unit-collectmetrics-0/charm/./src/charm.py:23: DeprecationWarning: don't use collect metrics
unit-collectmetrics-0: 17:44:14 WARNING unit.collectmetrics/0.update-status   framework.observe(self.on.collect_metrics, self._on_collect_metrics)

However, this would trigger on every event, not only the collect-metrics and *-series-upgrade ones, unless the check was more than event_kind in (...) and checked the event that was being handled, which isn't known inside of framework.observe. Maybe that's ok, since it draws more attention to this, and most likely in production they'll all be ignored anyway, but it would likely be a warning for every individual test if you're testing a charm that does observe one of these events.

[benhoyt]

Yep, I agree that stacklevel=3 seems best: good in tests, okay in production. Regarding moving this to observe -- yeah, I think where it is now (and keeping it restricted to these specific events) is best. Thanks!