Remove legacy source input, optional metricType query input for testing

[lmuhlha]

source was a legacy query option left over from when events were partially supported: https://github.com/spotify/heroic/commit/bc533078fad161efa76001cdc4f2a6e22dcb88d5#diff-0e5dda61cc1e44c094fed3ce698715de1d3b11747d9f312df0fbf6a52f70f1fcR222

This PR gets rid of source as an input and essentially re-names it to metricType with a few updates.

Since end users don't care about what metricType is being used, Distributions can instead set metricType based on what aggregation (tdigest) is being used.

[ao2017]

@lmuhlha
There is a concept of metric_type downstream that is completely different from the metric_type as used in this PR. metric_type upstream is ( histogram, counter, gauge ....) metric_type in heroic is ( Point , Group, DistributionPoint ...)

Unfortunately, in heroic api this 2 concepts co-exist. In the query you can make the distinction by using "source". If you replace source with metric_type, how are we going to ensure this change doesn't break anything?

I wanted to check but this branch is not compiling. Check this class => /Users/adeleo/src/heroic/heroic/heroic-dist/src/test/java/com/spotify/heroic/LoggingMetricModule.kt: (36, 19) .

[lmuhlha]

@ao2017 I fixed a few issues, now I'm just having some trouble with the tests. When i step through the code, the check for the tDigest aggregation instance sets the metricType properly. I noticed that there are tests that tDigest still works with no aggregation set. Is that how this is supposed to work? I think I'm a bit confused on functionality.

[lmuhlha]

@ao2017 as far as the naming, I just figured metricType matched what it is in Heroic. I don't think it will conflict with anything in semantic metrics. We can come up with another name if need be, i just think source was confusing as well.

[ao2017]

@ao2017 as far as the naming, I just figured metricType matched what it is in Heroic. I don't think it will conflict with anything in semantic metrics. We can come up with another name if need be, i just think source was confusing as well.

I am not sure why the name source was used :). If everything is working as expected I am good.

[ao2017]

@ao2017 I fixed a few issues, now I'm just having some trouble with the tests. When i step through the code, the check for the tDigest aggregation instance sets the metricType properly. I noticed that there are tests that tDigest still works with no aggregation set. Is that how this is supposed to work? I think I'm a bit confused on functionality.

Let's look into together. I will ping you.

[ao2017]

@ao2017 I fixed a few issues, now I'm just having some trouble with the tests. When i step through the code, the check for the tDigest aggregation instance sets the metricType properly. I noticed that there are tests that tDigest still works with no aggregation set. Is that how this is supposed to work? I think I'm a bit confused on functionality.

Let's look into together. I will ping you.

I checked. The refactoring on the consumer broke the ability to read distributionPoint from bigtable. It is causing query and consumer IT tests to fail. So the issue is not related to aggregation.

[codecov[bot]]

Codecov Report

Merging #749 (a18519f) into master (5e331a5) will decrease coverage by 0.00%. The diff coverage is 76.59%.

@@             Coverage Diff              @@
##             master     #749      +/-   ##
============================================
- Coverage     55.02%   55.02%   -0.01%     
+ Complexity     3154     3153       -1     
============================================
  Files           746      746              
  Lines         20386    20389       +3     
  Branches       1337     1339       +2     
============================================
+ Hits          11218    11219       +1     
- Misses         8681     8682       +1     
- Partials        487      488       +1     

Impacted Files	Coverage Δ	Complexity Δ
...main/java/com/spotify/heroic/metric/FullQuery.java	32.00% <0.00%> (ø)	7.00 <0.00> (ø)
...main/java/com/spotify/heroic/shell/task/Fetch.java	10.29% <0.00%> (ø)	2.00 <0.00> (ø)
...potify/heroic/metric/datastax/DatastaxBackend.java	27.27% <0.00%> (ø)	21.00 <0.00> (ø)
...potify/heroic/metric/bigtable/BigtableBackend.java	85.81% <50.00%> (ø)	61.00 <1.00> (ø)
...java/com/spotify/heroic/grammar/QueryExpression.kt	90.00% <66.66%> (ø)	10.00 <1.00> (ø)
...n/java/com/spotify/heroic/metric/QueryMetrics.java	81.81% <66.66%> (-0.80%)	4.00 <0.00> (ø)
...omponent/src/main/java/com/spotify/heroic/Query.kt	100.00% <100.00%> (ø)	7.00 <2.00> (ø)
...src/main/java/com/spotify/heroic/QueryBuilder.java	70.68% <100.00%> (+0.51%)	16.00 <2.00> (ø)
...c/main/java/com/spotify/heroic/metric/FetchData.kt	90.47% <100.00%> (ø)	3.00 <0.00> (ø)
...main/java/com/spotify/heroic/CoreQueryManager.java	75.53% <100.00%> (+0.31%)	10.00 <0.00> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5e331a5...a18519f. Read the comment docs.