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

dataclients/kubernetes: add metadata route - zone aware option 2 #2845

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlexanderYastrebov
Copy link
Member

@AlexanderYastrebov AlexanderYastrebov commented Jan 11, 2024

Adds a special route with a predefined id kube__metadata
that contains False predicate, no filters and endpoints metadata.

Endpoints metadata is encoded as JSON via data URI scheme
into the route backend address field.

Example eskip:

kube__metadata: False() -> "data:application/json;base64,eyJhZGRyZXNzZXMiO...";
...

A special route pre-processor detects and removes this route, decodes metadata
and stores it as EndpointRegistry metrics.

The endpoint metrics then used to obtain endpoint zone, node and pod names
for tracing and logging.

TODO:

  • add flag to enable metadata route
  • tests

Fixes #1559

@AlexanderYastrebov AlexanderYastrebov added the minor no risk changes, for example new filters label Jan 11, 2024
@AlexanderYastrebov AlexanderYastrebov force-pushed the dataclients/kubernetes/metadata-route branch 2 times, most recently from b773b22 to fa772eb Compare January 11, 2024 17:41
@szuecs
Copy link
Member

szuecs commented Jan 11, 2024

I don't like it. Maybe write first a doc for this and why we need metadata.
Why do we need a special route?
Why False() with input data?
Why we need so much data in that?
Why json and not something better?

@AlexanderYastrebov AlexanderYastrebov force-pushed the dataclients/kubernetes/metadata-route branch from 113fdb0 to fbae58c Compare January 12, 2024 12:59
@AlexanderYastrebov
Copy link
Member Author

AlexanderYastrebov commented Jan 12, 2024

I don't like it. Maybe write first a doc for this and why we need metadata.

Thank you for having a look. The idea is to use pod and node names in error logging and tracing. Zone could also be used to filter endpoints by zone before applying loadbalancing algorithm. Pod uid could be used to detect pod IP address reuse and e.g. reset endpoint detected time.
Data client embeds namespace into route id so it is less useful.

Why do we need a special route?

The biggest upside is that metadata about routing table is transferred along with the routing table itself so there is no other API or source of inconsistency.

Why False() with input data?

This was just a proof-of-concept. I changed the format now to use https://en.wikipedia.org/wiki/Data_URI_scheme

Why we need so much data in that?

We can decide what data of all we pass as metadata. We can start with pod name only and extend it later.

Why json and not something better?

JSON is extensible so we can add new fields later and separatetly on producer and consumer side. It is also a text format and there are a lot of tools to process it. We could use binary format but the data size is <200kB (and <30kB gzipped) so I do not think its worth the inconvenience.

@AlexanderYastrebov AlexanderYastrebov force-pushed the dataclients/kubernetes/metadata-route branch from fbae58c to 935cdd6 Compare January 12, 2024 14:28
@@ -24,3 +24,10 @@ type secret struct {
type secretList struct {
Items []*secret `json:"items"`
}

type objectReference struct {
Kind string `json:"kind"`
Copy link
Member

Choose a reason for hiding this comment

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

this is static and always "Pod", no need to store nor pass

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't know this and its not documented so I check kind. See https://pkg.go.dev/k8s.io/api/discovery/v1#Endpoint

Kind string `json:"kind"`
Name string `json:"name"`
Namespace string `json:"namespace"`
Uid string `json:"uid"`
Copy link
Member

Choose a reason for hiding this comment

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

this we don't need

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for unmarshalling k8s API response.
uid can be used to detect pod rotation with the same name, e.g. from StatefulSet.

@AlexanderYastrebov AlexanderYastrebov force-pushed the dataclients/kubernetes/metadata-route branch from 935cdd6 to d214c11 Compare January 12, 2024 17:13
@@ -1308,11 +1323,10 @@ func (p *Proxy) errorResponse(ctx *context, err error) {
uri = uri[:i]
}
logFunc(
`%s after %v, route %s with backend %s %s%s, status code %d: %v, remote host: %s, request: "%s %s %s", host: %s, user agent: "%s"`,
`%s after %v, route %s with backend %s%s, status code %d: %v, remote host: %s, request: "%s %s %s", host: %s, user agent: "%s"`,
Copy link
Member Author

Choose a reason for hiding this comment

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

This mess should be refactored into structured logging

@AlexanderYastrebov AlexanderYastrebov force-pushed the dataclients/kubernetes/metadata-route branch 2 times, most recently from 59373aa to 4de9f1a Compare January 13, 2024 21:24
Adds a special route with a predefined id `kube__metadata`
that contains `False` predicate, no filters and endpoints metadata.

Endpoints metadata is encoded as JSON via [data URI scheme](https://en.wikipedia.org/wiki/Data_URI_scheme)
into the route backend address field.

Example eskip:
```
kube__metadata: False() -> "data:application/json;base64,eyJhZGRyZXNzZXMiO...";
...
```

A special route pre-processor detects and removes this route, decodes metadata
and stores it as EndpointRegistry metrics.

The endpoint metrics then used to obtain endpoint node and pod names
for tracing and logging.

TODO:
- [ ] add flag  to enable metadata route
- [ ] tests

Fixes #1559

Signed-off-by: Alexander Yastrebov <[email protected]>
@AlexanderYastrebov AlexanderYastrebov force-pushed the dataclients/kubernetes/metadata-route branch from 4de9f1a to 5b46917 Compare January 17, 2024 10:10
@szuecs szuecs changed the title dataclients/kubernetes: add metadata route dataclients/kubernetes: add metadata route - zone aware option 2 Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor no risk changes, for example new filters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Endpoint metadata
2 participants