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

Option to enable che-server metrics endpoint #117

Merged
merged 8 commits into from
Nov 28, 2019

Conversation

sparkoo
Copy link
Member

@sparkoo sparkoo commented Nov 11, 2019

In eclipse-che/che#15046 and it's subtask eclipse-che/che#15137, we're going to implement way to deploy monitoring stack with che-operator. This PR is adding first step, to be able to enable metrics endpoint on server with che CR.

fixes eclipse-che/che#15210

@sparkoo
Copy link
Member Author

sparkoo commented Nov 11, 2019

changes of build_deploy_local.sh are in separate PR #116

@skabashnyuk
Copy link
Contributor

This pr is related to the discussion in eclipse-che/che#15136. Please do not merge until we have some agreement.

@sparkoo sparkoo changed the title Option to enable che-server metrics endpoint 🚧 Option to enable che-server metrics endpoint Nov 20, 2019
Signed-off-by: Michal Vala <mvala@redhat.com>
Signed-off-by: Michal Vala <mvala@redhat.com>

change cheMetrics to metrics.enable, remove metrics port, move che service creation to deploy package

Signed-off-by: Michal Vala <mvala@redhat.com>
Signed-off-by: Michal Vala <mvala@redhat.com>
Signed-off-by: Michal Vala <mvala@redhat.com>
# Conflicts:
#	pkg/deploy/che_configmap.go
@sparkoo sparkoo changed the title 🚧 Option to enable che-server metrics endpoint Option to enable che-server metrics endpoint Nov 27, 2019
Copy link
Contributor

@metlos metlos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from a minor nitpick in the tests.

t.Error("expected 2 ports")
}
checkPort(ports[0], "http", 8080, t)
checkPort(ports[1], "metrics", 8087, t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultCheMetricsPort instead of 8087?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I've also changed type from string to int32 to avoid conversions.

Signed-off-by: Michal Vala <mvala@redhat.com>
@sparkoo sparkoo merged commit a850317 into eclipse-che:master Nov 28, 2019
@sparkoo sparkoo deleted the metrics branch November 28, 2019 22:05
nickboldt pushed a commit to nickboldt/che-operator that referenced this pull request Jan 31, 2020
Signed-off-by: Michal Vala <mvala@redhat.com>
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.

[che-operator] - enable che-server monitoring endpoint
3 participants