Skip to content
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

Support connecting to multiple versions elasticsearch at runtime [beautified version of PR 2260] #2316

Merged
merged 2 commits into from Sep 25, 2018

Conversation

@bom-d-van
Copy link
Contributor

@bom-d-van bom-d-van commented Sep 24, 2018

With this patch bosun could connect to elasticsearch servers with different versions. Supported version are: 2.x, 5.x, 6.x. This is a breaking change, requiring users to update their configuration files to explicitly specify elasticsearch version with 2, 5, or 6, otherwise it would just panic (fail fast).

No compile and test errors on my machine, but I haven't test it against real elastic search servers. Publish it here to check out the community's opinions and the possibility of merging onto master. :)

Relevant tickets: #2240, #1982

Related PR: #2260

@bom-d-van bom-d-van force-pushed the bom-d-van:multi-es-spt branch from a9aa1f4 to fc03c9d Sep 24, 2018
@bom-d-van bom-d-van changed the title Support connecting to multiple versions elasticsearch at runtime [Beautified version of PR 2260] Support connecting to multiple versions elasticsearch at runtime [beautified version of PR 2260] Sep 24, 2018
@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 24, 2018

Quick tests against v5 seem to behave correctly.

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 24, 2018

Hmmm... I seem to be getting:

$ go run main.go -q -r -skiplast -dev  -n -w
2018/09/24 12:36:56 info: migrate.go:79: checking migrations
panic: interface conversion: interface {} is nil, not []elastic.ClientOptionFunc

goroutine 1 [running]:
main.main()
	/home/kbrandt/src/bosun.org/cmd/bosun/main.go:169 +0x18fe
exit status 2

That just me? Did not get that on the previous PR.

Edit: Seems to happen when I enable the AnnotateConf - maybe that was the case in the previous one as well.

@bom-d-van bom-d-van force-pushed the bom-d-van:multi-es-spt branch from fc03c9d to 2aed2ee Sep 24, 2018
@bom-d-van
Copy link
Contributor Author

@bom-d-van bom-d-van commented Sep 24, 2018

@kylebrandt Sorry, it was actually a bug in conf/system_elastic*.go files. I have pushed a fix: fdf3940

Commits in this PR have been reset too.

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 24, 2018

Can you reword the first commit to something like:

cmd/bosun: BREAKING-CHANGE: support multiple elasticsearch versions at runtime
- Elastic configuration in the system configuration now *requires* that Version be specified or will fail to start.
- bump version to 0.8.0
- ...whatever else you want to say...

Then rebase and merge time :-)

@bom-d-van bom-d-van force-pushed the bom-d-van:multi-es-spt branch from 2aed2ee to 0400704 Sep 25, 2018
bom-d-van added 2 commits Sep 25, 2018
…t runtime

- Elastic configuration in the system configuration now *requires* that Version be specified or will fail to start.
- Bump version to 0.8.0.
- Supported versions are: 2.x, 5.x, 6.x.
For supporting multiple versions of elasticsearch at runtime.
@bom-d-van bom-d-van force-pushed the bom-d-van:multi-es-spt branch from 0400704 to 6aec5dd Sep 25, 2018
@bom-d-van
Copy link
Contributor Author

@bom-d-van bom-d-van commented Sep 25, 2018

@kylebrandt Thanks, the new message is better. I tweak it a little bit. Hope that's ok.

@kylebrandt kylebrandt merged commit a40db73 into bosun-monitor:master Sep 25, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 25, 2018

Getting panics on this, not quite sure what it is yet ... "reflect: call with two few arguments" ... I wonder if maybe it is something to do with the prefix but not sure yet

@pradeepbbl
Copy link
Contributor

@pradeepbbl pradeepbbl commented Sep 25, 2018

I will have a look at the prefix key..

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 25, 2018

@bom-d-van
Copy link
Contributor Author

@bom-d-van bom-d-van commented Sep 25, 2018

@kylebrandt is there any stack trace? Not really sure what's going on.

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 25, 2018

panic: reflect: Call with too few input arguments [recovered]
	panic: reflect: Call with too few input arguments

goroutine 951 [running]:
bosun.org/cmd/bosun/expr.errRecover(0xc0012e9e80)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:144 +0x1b7
panic(0x105da60, 0x1607c40)
	/usr/local/go/src/runtime/panic.go:513 +0x1b9
reflect.Value.call(0x10d47e0, 0x1517b70, 0x13, 0x129d8c7, 0x4, 0xc0012e9058, 0x1, 0x1, 0xc00081e901, 0x10e0720, ...)
	/usr/local/go/src/reflect/value.go:364 +0x152f
reflect.Value.Call(0x10d47e0, 0x1517b70, 0x13, 0xc0012e9058, 0x1, 0x1, 0x0, 0x0, 0x2)
	/usr/local/go/src/reflect/value.go:308 +0xa4
bosun.org/cmd/bosun/expr.(*State).walkFunc.func1(0x1616840, 0xc000e74a20)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:779 +0xe70
bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler.(*Profile).Step(0xc000e74a20, 0xc000a5e6e0, 0xb, 0xc0005726e0)
	/home/kbrandt/src/bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler/types.go:195 +0x76
bosun.org/cmd/bosun/expr.(*State).walkFunc(0xc000e74990, 0xc000a1ec30, 0x1)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:731 +0xff
bosun.org/cmd/bosun/expr.(*State).walkFunc.func1(0x1616840, 0xc000e74a20)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:741 +0x11d7
bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler.(*Profile).Step(0xc000e74a20, 0xc000a5e6c0, 0xd, 0xc0005725c0)
	/home/kbrandt/src/bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler/types.go:195 +0x76
bosun.org/cmd/bosun/expr.(*State).walkFunc(0xc000e74990, 0xc000a1eb90, 0x0)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:731 +0xff
bosun.org/cmd/bosun/expr.(*State).walkFunc.func1(0x1616840, 0xc000e74a20)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:741 +0x11d7
bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler.(*Profile).Step(0xc000e74a20, 0xc000a5e6b0, 0x9, 0xc000572580)
	/home/kbrandt/src/bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler/types.go:195 +0x76
bosun.org/cmd/bosun/expr.(*State).walkFunc(0xc000e74990, 0xc000a1eb40, 0x0)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:731 +0xff
bosun.org/cmd/bosun/expr.(*State).walkFunc.func1(0x1616840, 0xc000e74a20)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:741 +0x11d7
bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler.(*Profile).Step(0xc000e74a20, 0xc000a5e6a8, 0x8, 0xc000572500)
	/home/kbrandt/src/bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler/types.go:195 +0x76
bosun.org/cmd/bosun/expr.(*State).walkFunc(0xc000e74990, 0xc000a1eaf0, 0xc000a5e681)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:731 +0xff
bosun.org/cmd/bosun/expr.(*State).walk(0xc000e74990, 0x1617c20, 0xc000a1eaf0, 0xc0011edae0)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:483 +0x13f
bosun.org/cmd/bosun/expr.(*State).walkBinary(0xc000e74990, 0xc0009f3f80, 0xc0000ba030)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:505 +0x5a
bosun.org/cmd/bosun/expr.(*State).walk(0xc000e74990, 0x1617b60, 0xc0009f3f80, 0x1)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:479 +0x3ed
bosun.org/cmd/bosun/expr.(*State).walkBinary(0xc000e74990, 0xc000a80000, 0x0)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:505 +0x5a
bosun.org/cmd/bosun/expr.(*State).walk(0xc000e74990, 0x1617b60, 0xc000a80000, 0x160e)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:479 +0x3ed
bosun.org/cmd/bosun/expr.(*State).walkBinary(0xc000e74990, 0xc000a80300, 0x40d7df)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:505 +0x5a
bosun.org/cmd/bosun/expr.(*State).walk(0xc000e74990, 0x1617b60, 0xc000a80300, 0x0)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:479 +0x3ed
bosun.org/cmd/bosun/expr.(*Expr).ExecuteState.func1(0x1616840, 0xc000e74a20)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:125 +0x4c
bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler.(*Profile).Step(0xc000e74a20, 0x12a6350, 0xc, 0xc0005724c0)
	/home/kbrandt/src/bosun.org/vendor/github.com/MiniProfiler/go/miniprofiler/types.go:195 +0x76
bosun.org/cmd/bosun/expr.(*Expr).ExecuteState(0xc000326bf0, 0xc000e74990, 0xc000566690, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:124 +0x10e
bosun.org/cmd/bosun/expr.(*Expr).Execute(0xc000326bf0, 0xc000333ee0, 0xc000e87e00, 0x0, 0x0, 0x2e981d78, 0xed33c3c98, 0x0, 0x0, 0x458800, ...)
	/home/kbrandt/src/bosun.org/cmd/bosun/expr/expr.go:114 +0xe7
bosun.org/cmd/bosun/sched.(*Schedule).executeExpr(0x20076a0, 0x0, 0x0, 0xc000ebcfc0, 0xc00071c7e0, 0xc000326bf0, 0x0, 0x0, 0x0)
	/home/kbrandt/src/bosun.org/cmd/bosun/sched/check.go:646 +0x177
bosun.org/cmd/bosun/sched.(*Schedule).CheckExpr.func2(0x20076a0, 0x0, 0x0, 0xc000ebcfc0, 0xc00071c7e0, 0xc000326bf0, 0xc00130e1e0)
	/home/kbrandt/src/bosun.org/cmd/bosun/sched/check.go:669 +0x6d
created by bosun.org/cmd/bosun/sched.(*Schedule).CheckExpr
	/home/kbrandt/src/bosun.org/cmd/bosun/sched/check.go:668 +0x135
exit status 2
@pradeepbbl
Copy link
Contributor

@pradeepbbl pradeepbbl commented Sep 25, 2018

@kylebrandt same here did run a couple of esquery from expression editor (with and without prefix) not able to reproduce this issue. I will run a test instance tomorrow and will update you in case of issue.

-Thanks

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 25, 2018

got it.... will push to master

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 25, 2018

Panic fixed in d400f79 , just a left over argument from the miniprofiler refactor (when it was moved the the State object)

@bom-d-van
Copy link
Contributor Author

@bom-d-van bom-d-van commented Sep 25, 2018

@kylebrandt Sorry for that. I relied on go build for the miniprofiler refactor that's why it wasn't caught. Thanks.

@kylebrandt
Copy link
Member

@kylebrandt kylebrandt commented Sep 25, 2018

@bom-d-van No apologies needed! Really appreciate your work here, this will make ES upgrade path so much better for many people, including at my company :P

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

Successfully merging this pull request may close these issues.

None yet

3 participants