Skip to content

Scripted metric aggregation support #1135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 12, 2014
Merged

Conversation

gmarz
Copy link
Contributor

@gmarz gmarz commented Dec 12, 2014

Adding support for the scripted metric aggeragtion

The tricky bit with mapping this aggregation is that the resulting value that the scripts return can be of different types and our current ValueMatric only supports the value being numeric (implemented as a double?).

As a solution, I came up with ScriptedValueMetric which holds the original object deserialized by Json.Net, and exposes ValueAs<T> which converts the value to the specified T. See these tests. Open to suggestions on a better way to handle this...

@Mpdreamz
Copy link
Member

LGTM @gmarz, like the workaround you came up with 👍

Made some small tweaks before merging it in.

The object Value prop is now internal, do not think we should expose JObject directly even if its boxed as object. Renamed ValueAs<>() to Value<>().

Mpdreamz added a commit that referenced this pull request Dec 12, 2014
@Mpdreamz Mpdreamz merged commit d4cf9e7 into develop Dec 12, 2014
@Mpdreamz Mpdreamz deleted the feature/aggs-scriptedmetric branch December 12, 2014 09:15
@gmarz
Copy link
Contributor Author

gmarz commented Dec 12, 2014

@Mpdreamz 👍 on making _Value internal. I actually had it that way originally, but was unsure.

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

Successfully merging this pull request may close these issues.

2 participants