Analyst 0.2.0 #2
Conversation
…ut any Presenter pieces and more
Codecov Report
@@ Coverage Diff @@
## develop #2 +/- ##
==========================================
+ Coverage 0.85% 9.65% +8.79%
==========================================
Files 29 216 +187
Lines 234 2818 +2584
==========================================
+ Hits 2 272 +270
- Misses 232 2546 +2314
Continue to review full report at Codecov.
|
|
@pauljohanneskraft Thanks for the improvements to Analyst! I took a look at the new Query API, it looks very nice! 🥳 Regarding the failing macOS builds. Unfortunately GitHub Actions currently don't support macOS 12 at the moment (actions/runner-images#3649) which would be the easy fix. As discussed you also have the GitHub permissions to merge the PR despite the SwiftLint warnings once you feel the PR is ready 🚀 |
Analyst 0.2.0
♻️ Current situation
The current Metrics Query API does not really allow an implementing metrics provider to specify, which operations are valid or not. Furthermore, it is not extensible to operations that one provider has and another does not.
Analyst at the moment features a dependency to Presenter. To reduce the clutter of unnecessary dependencies, it will be removed from this repository and moved to another (possibly Presenter directly or ApodiniAnalystPresenter).
Analyst features close to zero testing coverage.
Analyst has too many dependencies for minor features (such as a networking dependency, even though a system-one could be used).
Analyst can only be used for metrics and traces together.
💡 Proposed solution
To increase type safety in this regard, the new API is much more massive by defining separate types for each operator. These types can be made available to each provider depending on whether they make them conform to a protocol or not - See PrometheusAnalyst for an example of that.
PrometheusAnalyst should feature some tests with an active Prometheus instance (that is run from within the test).
Analyst gets rid of dependencies to Presenter, AsyncHTTPClient and NIO. It adds a (small) dependency to Swift-Metrics, however, since that makes the API of Collector and Analyst more consistent without unnecessarily bloating the framework with a lot of unused features.
There is now MetricAnalyst, TraceAnalyst and Analyst (which is simply MetricAnalyst+TraceAnalyst).
Implications
Refactorings that include interface changes. Most of the operations should be somewhat compatible with both systems, but they are not 1-to-1-translatable in some cases.
Users of this framework will need to add dependencies of AsyncHTTPClient and NIO, if they have not otherwise specified and are using their interface through Analyst at the moment.
Testing
Yes, some minor tests on whether the Metric Query API is working correctly for Prometheus (i.e. checking expected and computed strings). There is also a test that runs Prometheus itself and checks whether network requests can be sent and received successfully.
Reviewer Nudging
Take a look at the Metric Query API - first the folders "0 Operators", "1 Operator", "2 Operators" and some of its operators to understand how queries are now built - it is probably too difficult to check all operators separately, but having a look at the overall structure and understanding that makes a lot of sense.
Also, it makes sense to have a look at how Prometheus implements the MetricAnalyst operators (not checking all of them, but having a brief overview).