Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Implement log function natively #2021

Merged
merged 6 commits into from
Aug 11, 2022

Conversation

zlin179
Copy link
Contributor

@zlin179 zlin179 commented Jan 4, 2022

Describe your changes
Add the log function similar to Graphite's log

Testing performed

  • Unit Test (pass)
  • Integration Test (pass)

I tested using mtcmptest with the following json

{
    "log()": "log(seriesByTag('name=jitter.g'))",
    "log(0.8)": "log(seriesByTag('name=jitter.g'),0.8)",
    "log(2.73)": "log(seriesByTag('name=jitter.g'),2.73)",
    "log(10)": "log(seriesByTag('name=jitter.g'),10)",
    "log(233333333)": "log(seriesByTag('name=jitter.g'),233333333)"
}

Basically they are the same except that the tag for log is {string: string} in mt & {string: float} in graphite, and very occasionally there's 1 bit precision difference.
link to the comparison result

Additional context
Benchmark

goos: darwin
goarch: amd64
pkg: github.com/grafana/metrictank/expr
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkLog10k_1NoNulls
BenchmarkLog10k_1NoNulls-12                        39400             28115 ns/op
BenchmarkLog10k_10NoNulls
BenchmarkLog10k_10NoNulls-12                        4143            300366 ns/op
BenchmarkLog10k_100NoNulls
BenchmarkLog10k_100NoNulls-12                        342           3290549 ns/op
BenchmarkLog10k_1000NoNulls
BenchmarkLog10k_1000NoNulls-12                        37          30940075 ns/op
BenchmarkLog10k_1SomeSeriesHalfNulls
BenchmarkLog10k_1SomeSeriesHalfNulls-12            43148             27560 ns/op
BenchmarkLog10k_10SomeSeriesHalfNulls
BenchmarkLog10k_10SomeSeriesHalfNulls-12            4471            293390 ns/op
BenchmarkLog10k_100SomeSeriesHalfNulls
BenchmarkLog10k_100SomeSeriesHalfNulls-12            384           3079671 ns/op
BenchmarkLog10k_1000SomeSeriesHalfNulls
BenchmarkLog10k_1000SomeSeriesHalfNulls-12            40          29678756 ns/op
BenchmarkLog10k_1AllSeriesHalfNulls
BenchmarkLog10k_1AllSeriesHalfNulls-12             42213             28313 ns/op
BenchmarkLog10k_10AllSeriesHalfNulls
BenchmarkLog10k_10AllSeriesHalfNulls-12             4590            256792 ns/op
BenchmarkLog10k_100AllSeriesHalfNulls
BenchmarkLog10k_100AllSeriesHalfNulls-12             392           3157942 ns/op
BenchmarkLog10k_1000AllSeriesHalfNulls
BenchmarkLog10k_1000AllSeriesHalfNulls-12             37          30260522 ns/op
PASS
ok      github.com/grafana/metrictank/expr      18.915s

@zlin179
Copy link
Contributor Author

zlin179 commented Feb 10, 2022

We should change the signature to func benchmarkLog(b *testing.B, numSeries int, fn0, fn1 test.DataFunc) if #2017 gets merged first

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2022

CLA assistant check
All committers have signed the CLA.

@zlin179
Copy link
Contributor Author

zlin179 commented Jun 15, 2022

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

When I click it, it says

Error
There is no CLA to sign for grafana/metrictank

(API rate limit exceeded for user ID 1443603.)

@robert-milan
Copy link
Contributor

Hi @zlin179 . Can you try signing the CLA again, or possibly click on recheck it? If that doesn't help, possibly a rebase off of master?

@zlin179
Copy link
Contributor Author

zlin179 commented Aug 11, 2022

Hi @zlin179 . Can you try signing the CLA again, or possibly click on recheck it? If that doesn't help, possibly a rebase off of master?

Sure, signed it.

@robert-milan robert-milan merged commit 9a03ab6 into grafana:master Aug 11, 2022
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

4 participants