Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Adding New Iterator Transform: foreach #81

Closed
wants to merge 16 commits into from

Conversation

aertoria
Copy link
Contributor

@aertoria aertoria commented Jul 17, 2016

Adding New Iterator Transform: foreach
Syntax example:
OLD: SUM(SCALE(-10d:M:cpu{device=*}:avg, -10d:M:count{device=*}:avg))
NEW: SUM(foreach(-10d:M:cpu{device=*}:avg,-10d:M:count{device=*}:avg,$device,$SCALE))

Scope: Can apply on any existing TRANSFORM with their existing constants requirement if any.

Detail:
foreach is implemented against a new type "TRANSFORMITERATOR", which extends current type "TRANSFORM". This architecture allows "foreach" being treated same way in metricReader and factory. However, they are different in that:

CURRENT TYPE Transform.transform: taking Array<Metric>, giving Array<Metric>
NEW TYPE TransformIterator.iterate: taking Array<Metric>, giving Array<TransformExecutable>

in above, each TransformExecutable is Array<Metric>, so they that can be consumed directly by any Transform.transform

@codecov-io
Copy link

codecov-io commented Jul 17, 2016

Current coverage is 36.07% (diff: 80.39%)

Merging #81 into master will increase coverage by 0.12%

@@             master        #81   diff @@
==========================================
  Files           225        226     +1   
  Lines         12585      12636    +51   
  Methods           0          0          
  Messages          0          0          
  Branches       2131       2142    +11   
==========================================
+ Hits           4524       4558    +34   
- Misses         7490       7500    +10   
- Partials        571        578     +7   

Powered by Codecov. Last update 3a9a0f2...7c0a97c

@aertoria
Copy link
Contributor Author

@saaja-sfdc
Let me know your thought. Thanks.

@aertoria
Copy link
Contributor Author

aertoria commented Aug 8, 2016

@saaja-sfdc @bsura
Update. Below is a dashboard that illustrates the usage of transform 'foreach'. It is right now running at DBClouds' private Argus and it becomes one of those very popular transforms since the release months back. Hope it's helpful.

after login
http://adhoc-db1-1-crd.eng.sfdc.net:8020/app/#/dashboards/308501

usage:
SUM(1464803454:1466045054:p90_sandbox:podperc90{podId=*}:avg,1464803454:1466045054:p90_sandbox:podperc90{podId=*}:avg)
versus
foreach(1464803454:1466045054:p90_sandbox:podperc90{podId=*}:avg,1464803454:1466045054:p90_sandbox:podperc90{podId=*}:avg,$podId,$SUM)

You may keep chaining other transform with any constants after that. Let me know if it helps.

@bsura
Copy link
Contributor

bsura commented Sep 15, 2016

@aertoria Can we close this pull request? We already have a GroupBy Transform that should be able to achieve this. Also if we are adding this Transform, then it should probably be done the way we had discussed it the other day.

@aertoria
Copy link
Contributor Author

As we discussed the other one will be ready for you to pull in about 1 week. But before that if you have something ready to ship then just close this one. Thanks

@bsura
Copy link
Contributor

bsura commented Sep 15, 2016

Check out this PR which has already been submitted.
#152

@bsura
Copy link
Contributor

bsura commented Sep 15, 2016

Closing this one.

@bsura bsura closed this Sep 15, 2016
@aertoria
Copy link
Contributor Author

Thanks

@aertoria
Copy link
Contributor Author

@cksassy

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants