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.

Sign up for GitHub

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

Add support for customized monitoring API #22605

Merged
merged 6 commits into from
Dec 21, 2020
Merged

Add support for customized monitoring API #22605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b358184
allow customized monitoring api to be added
newly12 Nov 17, 2020
bfbed50
update CHANGELOG.next.asciidoc
newly12 Nov 17, 2020
b1d3bc9
support register/deregister routes dynamically and refactor test case
newly12 Dec 11, 2020
ebc290c
Revert "support register/deregister routes dynamically and refactor t…
newly12 Dec 16, 2020
8222f53
check handler func existence before adding it
newly12 Dec 16, 2020
340a67a
Merge branch 'master' into customzied_api
newly12 Dec 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Added new `rate_limit` processor for enforcing rate limits on event throughput. {pull}22883[22883]
- Allow node/namespace metadata to be disabled on kubernetes metagen and ensure add_kubernetes_metadata honors host {pull}23012[23012]
- Honor kube event resysncs to handle missed watch events {pull}22668[22668]
- Add support for customized monitoring API. {pull}22605[22605]

*Auditbeat*

Expand Down
15 changes: 15 additions & 0 deletions libbeat/api/routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import (
type handlerFunc func(http.ResponseWriter, *http.Request)
type lookupFunc func(string) *monitoring.Namespace

var handlerFuncMap = make(map[string]handlerFunc)

// NewWithDefaultRoutes creates a new server with default API routes.
func NewWithDefaultRoutes(log *logp.Logger, config *common.Config, ns lookupFunc) (*Server, error) {
mux := http.NewServeMux()
Expand All @@ -38,6 +40,10 @@ func NewWithDefaultRoutes(log *logp.Logger, config *common.Config, ns lookupFunc
mux.HandleFunc("/state", makeAPIHandler(ns("state")))
mux.HandleFunc("/stats", makeAPIHandler(ns("stats")))
mux.HandleFunc("/dataset", makeAPIHandler(ns("dataset")))
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this PR is already merged but I just noticed this while reviewing the backport PR. Should these three API routes be added via AddHandlerFunc too? That would ensure that someone doesn't try to override these routes when registering a custom one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comments.

As mentioned in this reply, the usage of AddHandlerFunc would be registering other routes in a module's "init" phase, that makes customized routes to be shown up before "default" routes, and the reason "default" routes can not be done in "init" phase is the handler functions relies on ns lookupFunc which is passed only in NewWithDefaultRoutes.

I think what can be done is

  • call AddHandlerFunc for default routes as well so at least errors can be logged when there are conflicts.
  • or move default routes registration to init phase and hardcode the ns to be monitoring.GetNamespace and change the interface of NewWithDefaultRoutes .

Any thoughts?


for api, h := range handlerFuncMap {
mux.HandleFunc(api, h)
}
return New(log, mux, config)
}

Expand Down Expand Up @@ -73,3 +79,12 @@ func prettyPrint(w http.ResponseWriter, data common.MapStr, u *url.URL) {
fmt.Fprintf(w, data.String())
}
}

// AddHandlerFunc provides interface to add customized handlerFunc
func AddHandlerFunc(api string, h handlerFunc) error {
if _, exist := handlerFuncMap[api]; exist {
return fmt.Errorf("%s already exist", api)
}
handlerFuncMap[api] = h
Copy link

Choose a reason for hiding this comment

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

We should error/panic if the api is already in use. A new handlerFunc should not be allowed to replace an older handlerFunc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now considered in the new implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted to previous implementation and added check in this function..

return nil
}