-
Notifications
You must be signed in to change notification settings - Fork 241
Rename metrics to monitoring to support additional routes #1640
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
Conversation
f079a84 to
919495d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1640 +/- ##
=======================================
Coverage 99.95% 99.95%
=======================================
Files 337 337
Lines 29504 29524 +20
=======================================
+ Hits 29492 29512 +20
Misses 8 8
Partials 4 4 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
bf6ad5a to
6ada040
Compare
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
…o monitoring-routes
9998013 to
d2e0652
Compare
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
d2e0652 to
eb60b6c
Compare
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
EnriqueL8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @Chengxuan - We will need to add this to the release notes for folks to be aware of
| go func() { | ||
| if err := server.ListenAndServeTLS(publicKeyFile.Name(), privateKeyFile.Name()); err != nil && err != http.ErrServerClosed { | ||
| log.Fatalf("ListenAndServeTLS(): %v", err) | ||
| } | ||
| }() | ||
|
|
||
| // Wait for the server to be ready | ||
| for { | ||
| conn, err := tls.Dial("tcp", server.Addr, &tls.Config{ | ||
| InsecureSkipVerify: true, | ||
| }) | ||
| if err == nil { | ||
| conn.Close() | ||
| break | ||
| } | ||
| time.Sleep(10 * time.Millisecond) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this Cheng!
Proposed changes
As elaborated in hyperledger/firefly-common#167, there is a need to expose more endpoints for operational checks.
This PR propose a new
monitoringsections in the configuration file for configurations of the monitoring server:metricsPathThe new monitoring server itself is a replacement for the existing metrics server.
As a result the existing
metricssection is marked as deprecated.Adding monitoring routes for the latest best practices. The existing
metricsconfig is marked as deprecated.The FireFly API path has been split into
apiandspiand then namespacedapi. So adopting the common utils will need some dedicated time, which I didn't manage to do.Types of changes
Please make sure to follow these points
< issue name >egAdded links in the documentation.Screenshots (If Applicable)
Other Information
Any message for the reviewer or kick off the discussion by explaining why you considered this particular solution, any alternatives etc.