refactor(logging): Clean up statsd logging

[sebastianliebscher]

SUMMARY

This PR removes the log_to_statsd= functionality from the @event_logger and replaces usage with already existing @statsd_metrics decorator. I basically moved BaseSupersetApiMixin from base_api.py --> base.py so that the flask views can use @statsd_metrics too.

Currently, there are two decorators for basically the same use case:

• @event_logger.log_this_with_context(log_to_statsd=True)
• @statsd_metrics

The @event_logger just increments a statsd counter, while @statsd_metrics additionally measures the execution time of a function.

TESTING INSTRUCTIONS

• run unit + integration tests
• manually checked for new statsd metrics

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[sebastianliebscher]

Most API endpoints already use @statsd_metrics. When this decorator is added to replace implicit log_to_statsd=True like in this example, the statsd metric

• superset_<Endpoint>

will be replaced by

• superset_<Class>_<Endpoint>_success|warning|error
• superset_<Class>_<Endpoint>_time
• superset_<Class>_<Endpoint>_time_count
• superset_<Class>_<Endpoint>_time_sum

So it is kind of a breaking change.

[john-bodley]

Thanks @sebastianliebscher for the refactor/cleanup. Given (as you mentioned) this is a breaking change, i.e., the logged statsD metric name will change it seems like we would have to wait until the breaking window opens for 4.0 (circa Q1 2024) before merging.

In the interim I was wondering whether there is a workaround? One could double log the metric (which does pollute the metric space) and add deprecation logic in 3.1 (slated for late Q4 2023) and then remove the double logging in 4.0.

cc @michael-s-molina

[michael-s-molina]

One could double log the metric (which does pollute the metric space) and add deprecation logic in 3.1 (slated for late Q4 2023) and then remove the double logging in 4.0.

@john-bodley This seems like too much for something that is not urgent. I think it would be better to just wait for the breaking window.

[rusackas]

Putting this one on our 5.0 project board as well. This one looks like it needs a big ol' rebase, though, at some point before October-ish.

[rusackas]

I'll just convert it to Draft for now, but please feel free to mark it as "Ready for Review" when it's rebased.