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

[Added] account specific monitoring endpoint(s) #3250

Merged
merged 3 commits into from Jul 12, 2022
Merged

Conversation

matthiashanel
Copy link
Contributor

@matthiashanel matthiashanel commented Jul 8, 2022

Added http monitoring endpoint /accstatz
It responds with a list of statz for all accounts with local connections
the argument "unused=1" can be provided to get statz for all accounts
This endpoint is also exposed as nats request under:

This monitoring endpoint is exposed via the system account.
$SYS.REQ.ACCOUNT.*.STATZ
Each server will respond with connection statistics for the requested
account. The format of the data section is a list (size 1) identical to the event
$SYS.ACCOUNT.%s.SERVER.CONNS which is sent periodically as well as on
connect/disconnect. Unless requested by options, server without the account,
or server where the account has no local connections, will not respond.

A PING endpoint exists as well. The response format is identical to
$SYS.REQ.ACCOUNT.*.STATZ
(however the data section will contain more than one account, if they exist)
In addition to general filter options the request takes a list of accounts and
an argument to include accounts without local connections (disabled by default)
$SYS.REQ.ACCOUNT.PING.STATZ

Each account has a new system account import where the local subject
$SYS.REQ.ACCOUNT.PING.STATZ essentially responds as if
the importing account name was used for $SYS.REQ.ACCOUNT.*.STATZ

The only difference between requesting ACCOUNT.PING.STATZ from within
the system account and an account is that the later can only retrieve
statz for the account the client requests from.

Also exposed the monitoring /healthz via the system account under
$SYS.REQ.SERVER.*.HEALTHZ
$SYS.REQ.SERVER.PING.HEALTHZ
No dedicated options are available for these.
HEALTHZ also accept general filter options.

Signed-off-by: Matthias Hanel mh@synadia.com

@matthiashanel matthiashanel force-pushed the acc-statz branch 14 times, most recently from ce780f6 to 1f345b8 Compare July 11, 2022 19:22
@matthiashanel matthiashanel marked this pull request as ready for review July 11, 2022 21:18
server/events.go Outdated Show resolved Hide resolved
// Do not leak too much here.
return nil, fmt.Errorf("bad request")
}
optz.ConnzOptions.isAccountReq = true
Copy link
Member

Choose a reason for hiding this comment

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

Since this is removed, the whole field isAccountReq in ConnzOptions has no relevance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted isAccountReq and it's usage.
I believe it's only occurrence was redundant with account being set or not.
That usage adjusted the Total, and I believe this should happen even when the request for an account comes from within the system account.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deleted isAccountReq and it's usage.
I believe it's only occurrence was redundant with account being set or not.
That usage adjusted the Total, and I believe this should happen even when the request for an account comes from within the system account.

server/events.go Show resolved Hide resolved
server/events.go Outdated Show resolved Hide resolved
server/events.go Outdated
@@ -1681,6 +1727,25 @@ func (s *Server) sendAccConnsUpdate(a *Account, subj ...string) {
a.mu.Unlock()
}

// Lock shoulc be held on entry
func (a *Account) accConns() *AccountStat {
Copy link
Member

Choose a reason for hiding this comment

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

Since this is an Account, I would rename accConns() - since we know it is an account - to either conns() or connStats() or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed to statz as to not overlap with stats inside the account

@kozlovic
Copy link
Member

@matthiashanel You may want to do a rebase from updated main. After that, I can have a last look and LGTM it!

Added account specific statz endpoint to the system account
$SYS.REQ.ACCOUNT.*.STATZ
Each server will respond with connection statistics for the requested
account. The format of the data section is identical to the event
$SYS.ACCOUNT.%s.SERVER.CONNS which is sent periodically as well as on
connect/disconnect.

A PING endpoint exists as well. There each server will respond with one
message per (requested) account. The response format is identical to
$SYS.REQ.ACCOUNT.*.STATZ
In addition to general filter options
the request takes a list of accounts and an argument to include accounts
without local connections (disabled by default)
$SYS.REQ.ACCOUNT.PING.STATZ

Each account has a new system account import where the local subject
$SYS.REQ.ACCOUNT.PING.STATZ essentially responds as if
the importing account name was used for SYS.REQ.ACCOUNT.*.STATZ

The only difference between requesting ACCOUNT.PING.STATZ from within
the system account and an account is that the later can only retrieve
statz for the local account.

Furthermore added http monitoring endpoint /accstatz
It responds with a list of statz for all accounts with local connections
the argument "unused=1" can be provided to get statz for all accounts
This endpoint is also exposed as nats request under:
$SYS.REQ.SERVER.*.ACC_STATZ
$SYS.REQ.SERVER.PING.ACC_STATZ

Also exposed the monitoring /healthz via the system account under
$SYS.REQ.SERVER.*.HEALTHZ
$SYS.REQ.SERVER.PING.HEALTHZ
No dedicated options are available for these.

ACC_STATZ and HEALTHZ also accept general filter options.

Signed-off-by: Matthias Hanel <mh@synadia.com>
Signed-off-by: Matthias Hanel <mh@synadia.com>
Signed-off-by: Matthias Hanel <mh@synadia.com>
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM

@matthiashanel matthiashanel merged commit d53d2d0 into main Jul 12, 2022
@matthiashanel matthiashanel deleted the acc-statz branch July 12, 2022 19:50
matthiashanel added a commit that referenced this pull request Jul 14, 2022
Added http monitoring endpoint /accstatz
It responds with a list of statz for all accounts with local connections
the argument "unused=1" can be provided to get statz for all accounts
This endpoint is also exposed as nats request under:

This monitoring endpoint is exposed via the system account.
$SYS.REQ.ACCOUNT.*.STATZ
Each server will respond with connection statistics for the requested
account. The format of the data section is a list (size 1) identical to the event
$SYS.ACCOUNT.%s.SERVER.CONNS which is sent periodically as well as on
connect/disconnect. Unless requested by options, server without the account,
or server where the account has no local connections, will not respond.

A PING endpoint exists as well. The response format is identical to
$SYS.REQ.ACCOUNT.*.STATZ
(however the data section will contain more than one account, if they exist)
In addition to general filter options the request takes a list of accounts and
an argument to include accounts without local connections (disabled by default)
$SYS.REQ.ACCOUNT.PING.STATZ

Each account has a new system account import where the local subject
$SYS.REQ.ACCOUNT.PING.STATZ essentially responds as if
the importing account name was used for $SYS.REQ.ACCOUNT.*.STATZ

The only difference between requesting ACCOUNT.PING.STATZ from within
the system account and an account is that the later can only retrieve
statz for the account the client requests from.

Also exposed the monitoring /healthz via the system account under
$SYS.REQ.SERVER.*.HEALTHZ
$SYS.REQ.SERVER.PING.HEALTHZ
No dedicated options are available for these.
HEALTHZ also accept general filter options.

Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel added a commit that referenced this pull request Aug 19, 2022
When a request for a system service like $SYS.REQ.ACCOUNT.*.CONNZ
is imported/exported we ensured that the requesting account is identical
to the account referenced in the subject.

In #3250 this check was extended from CONNZ to all $SYS.REQ.ACCOUNT.*.*
requests.

In general this check interferes with monitoring accounts that need
to query all other accounts, not just itself.
There the use case is that account A sends a request with account B
in the subject. The check for equal accounts prevents this.

This change removes the check to support these use cases.

Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel added a commit that referenced this pull request Aug 19, 2022
When a request for a system service like $SYS.REQ.ACCOUNT.*.CONNZ
is imported/exported we ensured that the requesting account is identical
to the account referenced in the subject.

In #3250 this check was extended from CONNZ to all $SYS.REQ.ACCOUNT.*.*
requests.

In general this check interferes with monitoring accounts that need
to query all other accounts, not just itself.
There the use case is that account A sends a request with account B
in the subject. The check for equal accounts prevents this.

This change removes the check to support these use cases.

Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel added a commit that referenced this pull request Aug 19, 2022
When a request for a system service like $SYS.REQ.ACCOUNT.*.CONNZ
is imported/exported we ensured that the requesting account is identical
to the account referenced in the subject.

In #3250 this check was extended from CONNZ to all $SYS.REQ.ACCOUNT.*.*
requests.

In general this check interferes with monitoring accounts that need
to query all other accounts, not just itself.
There the use case is that account A sends a request with account B
in the subject. The check for equal accounts prevents this.

This change removes the check to support these use cases.

Instead of the check, the default export now uses exportAuth
tokenPos to ensure that the 4th token is the importer account id.
This guarantees that an explicit export (done by user) can only import
for the own account.

This change also ensures that an explicit export is not overwritten
by the system.
This is not a problem when the export is public.
Automatic imports set the account id correctly and do not use wildcards.

To cover cases where the export is private, automatically added imports
are not subject a token check.

Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel added a commit that referenced this pull request Aug 19, 2022
When a request for a system service like $SYS.REQ.ACCOUNT.*.CONNZ
is imported/exported we ensured that the requesting account is identical
to the account referenced in the subject.

In #3250 this check was extended from CONNZ to all $SYS.REQ.ACCOUNT.*.*
requests.

In general this check interferes with monitoring accounts that need
to query all other accounts, not just itself.
There the use case is that account A sends a request with account B
in the subject. The check for equal accounts prevents this.

This change removes the check to support these use cases.

Instead of the check, the default export now uses exportAuth
tokenPos to ensure that the 4th token is the importer account id.
This guarantees that an explicit export (done by user) can only import
for the own account.

This change also ensures that an explicit export is not overwritten
by the system.
This is not a problem when the export is public.
Automatic imports set the account id correctly and do not use wildcards.

To cover cases where the export is private, automatically added imports
are not subject a token check.

Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel added a commit that referenced this pull request Aug 20, 2022
When a request for a system service like $SYS.REQ.ACCOUNT.*.CONNZ
is imported/exported we ensured that the requesting account is identical
to the account referenced in the subject.

In #3250 this check was extended from CONNZ to all $SYS.REQ.ACCOUNT.*.*
requests.

In general this check interferes with monitoring accounts that need
to query all other accounts, not just itself.
There the use case is that account A sends a request with account B
in the subject. The check for equal accounts prevents this.

This change removes the check to support these use cases.

Instead of the check, the default export now uses exportAuth
tokenPos to ensure that the 4th token is the importer account id.
This guarantees that an explicit export (done by user) can only import
for the own account.

This change also ensures that an explicit export is not overwritten
by the system.
This is not a problem when the export is public.
Automatic imports set the account id correctly and do not use wildcards.

To cover cases where the export is private, automatically added imports
are not subject a token check.

Signed-off-by: Matthias Hanel <mh@synadia.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.

None yet

2 participants