search

nats-io / nats-architecture-and-design

Architecture and Design Docs
Apache License 2.0
206 stars 22 forks source link

service framework - metadata options #206

Open aricart opened 1 year ago

aricart commented 1 year ago

Overview

Micro ADR-32 now allows optional metadata to be specified on the the service and on endpoints. The metadata is simply Record<string,string>. Note that schema report also includes any metadata associated with the endpoint.

The cross-client test has been updated (on the dev branch) if you are using it, it will flag if metadata is not specified in your test service.

Clients and Tools

Other Tasks

Client authors please update with your progress. If you open issues in your own repositories as a result of this request, please link them to this one by pasting the issue URL in a comment or main issue description.

giceratops commented 3 months ago

Whilst playing with the metadata in the Java impl I bumped into the following:

I wanted to change/add metadata after the client was created. Seemed to be a nice way to have dynamic info available of each service from the monitor. Without making endpoints, and calling those based on the results of a discovery. Similar like endpoints in stats.

This doesn't work because the ServiceRespsonse gets serialized at the Service constructor with the metadata originally provided at the builder. Atleast, the ones for ping and info do.

For the stats context the story is different. When passing an empty map to the builder it's nulled in the Service constructor, losing the original pointer. But by adding any (garbage) value to the map before building, the Service keeps the pointer to the original map (which then actually comes from the reference of the cached ping ServiceRespsonse before it is seriaziled). This way it is possible to expose some dynamic metadata using just the monitor client in the stats context. But this seems wrong?

Somehow it feels like it makes sense to have ping and info static, where stats is like endpoints. But in that case the isEmpty() should be removed to make its behaviour consistent? Or am I down a wrong path?

ripienaar commented 3 months ago

The metadata describes the service instance it’s not for dynamic use.

We are clarifying that in the spec and will enforce it in the clients.

Additionally we are adding a feature that allows you to build way to create an endpoint that expose your own metadata and that can be queried same as info in a 1;n manner.

See https://github.com/nats-io/nats-architecture-and-design/pull/300 @giceratops