Skip to content

add prometheus exporter for minecraft#25

Open
CasparChou wants to merge 4 commits into
itzg:masterfrom
CasparChou:features/prometheus-exporter
Open

add prometheus exporter for minecraft#25
CasparChou wants to merge 4 commits into
itzg:masterfrom
CasparChou:features/prometheus-exporter

Conversation

@CasparChou

@CasparChou CasparChou commented Nov 2, 2020

Copy link
Copy Markdown

add prometheus exporter for minecraft

visualization game stats and jvm memory usage

by using Prometheus Operator (optional) and Prometheus Exporter Plugins (https://dev.bukkit.org/projects/prometheus-exporter)

@CasparChou

CasparChou commented Nov 2, 2020

Copy link
Copy Markdown
Author

@itzg don't know how to fix lint-test failed... is there any more detail logs can check ?

@itzg

itzg commented Nov 2, 2020

Copy link
Copy Markdown
Owner

Great contribution; however, I would prefer that it incorporate

https://github.com/itzg/mc-monitor

using its Prometheus exporter mode since that will work for all server types without any additional plugin.

@CasparChou

Copy link
Copy Markdown
Author

mc-monitor seems cool !
maybe I can try to add this to chart

@itzg

itzg commented Nov 2, 2020

Copy link
Copy Markdown
Owner

Let me know if it looks like it'll be too hard to add mc-monitor and the changes you have will be an excellent step in that direction.

@CasparChou

CasparChou commented Nov 6, 2020

Copy link
Copy Markdown
Author

do you prefer to add a chart mc-monitor inside this repo, or just add a sidecar beside the minecraft-server container ?

@itzg

itzg commented Nov 6, 2020

Copy link
Copy Markdown
Owner

Since mc-monitor doesn't know how to do service discovery, then deploying as a side car makes more sense.

@CasparChou

CasparChou commented Nov 9, 2020

Copy link
Copy Markdown
Author

itzg/mc-monitor crash after pod started, but it works when I run the code by go run, still troubleshooting

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x7e1787]
goroutine 14 [running]:
main.(*promBedrockCollector).Collect(0xc000099a40, 0xc000172300)
/home/circleci/project/prom_collector.go:159 +0x357
main.promCollectors.Collect(0xc0000ad9c0, 0x2, 0x2, 0xc000172300)
/home/circleci/project/prom_collector.go:51 +0x64
github.com/prometheus/client_golang/prometheus.(*Registry).Gather.func1()
/go/pkg/mod/github.com/prometheus/client_golang@v1.5.1/prometheus/registry.go:443 +0x19d
created by github.com/prometheus/client_golang/prometheus.(*Registry).Gather
/go/pkg/mod/github.com/prometheus/client_golang@v1.5.1/prometheus/registry.go:535 +0xe36

it said prom_collector.go:L159 has a null pointer error
the problem maybe is mc server not ready

@itzg

itzg commented Nov 9, 2020

Copy link
Copy Markdown
Owner

Oh yeah, that might be an edge case that the code isn't handling well. I'll try to look at that today.

If I can get to that soon we can go with your original plan and then use a separate PR to add mc-monitor.

@itzg

itzg commented Nov 9, 2020

Copy link
Copy Markdown
Owner

Even IntelliJ is warning me just by it looking at the code :)

image

@CasparChou

Copy link
Copy Markdown
Author

lol. glad to find out the potential error

@itzg

itzg commented Nov 9, 2020

Copy link
Copy Markdown
Owner

0.6.1 now includes the fix

@ChipWolf

Copy link
Copy Markdown
Contributor

@CasparChou are you wanting to continue with this? Happy to take over if not

@caseyjmorton

Copy link
Copy Markdown

FYI to all interested in this, I'm going to resume work on this as part of my work on a minecraft-operator. Most of the operator code is based on these helm charts, so I'm happy to do any maintenance needed. That said, since this PR is almost 3 years old, I'm sure there has been plenty of drift and rather than try to resolve the conflicts, I'm just going to submit a totally new PR. Once filed, I will post a link to the new one here, for posterity.

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.

4 participants