Skip to content

natssysclient: add server-scoped monitoring endpoints#55

Open
piotrpio wants to merge 1 commit into
mainfrom
natssysclient/server-monitoring
Open

natssysclient: add server-scoped monitoring endpoints#55
piotrpio wants to merge 1 commit into
mainfrom
natssysclient/server-monitoring

Conversation

@piotrpio
Copy link
Copy Markdown
Collaborator

@piotrpio piotrpio commented Apr 17, 2026

Adds clients for missing SRV monitoring endpoints the library didn't previously cover, each exposed with a single-server method and a cluster-wide *Ping method

  • Idz / IdzPing
  • Routez / RoutezPing
  • Gatewayz / GatewayzPing
  • Leafz / LeafzPing
  • Accountz / AccountzPing
  • Profilez / ProfilezPing
  • Expvarz / ExpvarzPing
  • Ipqueuesz / IpqueueszPing
  • Raftz / RaftzPing

Signed-off-by: Piotr Piotrowski piotr@synadia.com

Adds clients for IDZ, ROUTEZ, GATEWAYZ, LEAFZ, ACCOUNTZ, PROFILEZ,
EXPVARZ, IPQUEUESZ, and RAFTZ, each with single-server and cluster
PING variants. Response and option types mirror nats-server's
server/monitor.go.

EXPVARZ, IPQUEUESZ, and RAFTZ require nats-server 2.11+; the README
marks them accordingly.

Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
@piotrpio piotrpio force-pushed the natssysclient/server-monitoring branch from 4bb7381 to 30fcc46 Compare April 17, 2026 20:37
@piotrpio
Copy link
Copy Markdown
Collaborator Author

@claude review this PR

@synadia-claude-reviewer
Copy link
Copy Markdown

synadia-claude-reviewer Bot commented Apr 17, 2026

Claude finished @piotrpio's task in 2m 8s —— View job


PR Review: natssysclient: add server-scoped monitoring endpoints

  • Read changed files and gather context
  • Review API design and consistency
  • Review implementation correctness
  • Review tests
  • Post review feedback

Overall this is a well-structured, comprehensive addition that faithfully follows existing patterns. Tests are thorough and include both single-server and cluster-wide (Ping) variants. The implementation is clean and consistent. A few observations below.


Issues

1. Pre-existing typo in ErrInvalidServerID — worth fixing while touching the file

api.go:41:

ErrInvalidServerID = errors.New("sever with given ID does not exist")

"sever" should be "server". This is pre-existing but the PR already modifies api.go, so it's a good opportunity to fix.

Fix this →


2. Embedding order inconsistency in IpqueueszEventOptions and RaftzEventOptions

ipqueuesz.go:48-51 and raftz.go:82-85:

// Inconsistent — EventFilterOptions first
IpqueueszEventOptions struct {
    EventFilterOptions
    IpqueueszOptions
}

RaftzEventOptions struct {
    EventFilterOptions
    RaftzOptions
}

Every other *EventOptions type in the package places the domain-specific options first:

// Consistent pattern from routez.go, gatewayz.go, leafz.go, accountz.go, etc.
RoutezEventOptions struct {
    RoutezOptions
    EventFilterOptions
}

This doesn't affect correctness (JSON marshaling is field-name based, not order-based), but it's inconsistent with the established pattern.

Fix this →


Minor Notes

ProfilezOptions.Duration serializationtime.Duration serializes as a nanosecond int64 in JSON, which matches what the NATS server expects. No issue, just confirming this is intentional and correct.

pingServers capacity pre-allocation (api.go) — the new if s.opts.serverCount > 0 { msgs = make(..., serverCount) } is a nice micro-optimization, good addition.

IdzPing takes no options — correct, since IDZ has no configurable parameters. The asymmetry with other *Ping methods (which all take an options struct) is expected and intentional here.

SubjectPermission defined in routez.go — it's also reused in leafz.go. Since both files are in the same package this is fine, though a dedicated types.go would be an alternative. Not a blocker.


Summary

The PR is clean, well-tested, and follows existing conventions. The two issues above are worth addressing — the typo is a definite fix and the embedding-order inconsistency keeps the API surface predictable. Everything else looks good to go.

@piotrpio piotrpio requested a review from Jarema April 20, 2026 14:15
@piotrpio piotrpio marked this pull request as ready for review April 20, 2026 14:27
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.

1 participant