[Telemetry] Prepare for versioned HTTP APIs #152111
Conversation
jloleysens
added
Team:Core
jloleysens
changed the title
|
Pinging @elastic/kibana-core (Team:Core) |
| @@ -33,7 +34,17 @@ export function registerTelemetryUserHasSeenNotice(router: IRouter) { | |||
| }; | |||
| await updateTelemetrySavedObject(soClient, updatedAttributes); | |||
|
|
|||
| return res.ok({ body: updatedAttributes }); | |||
| const body: v2.Telemetry = { | |||
| allowChangingOptInStatus: updatedAttributes.allowChangingOptInStatus, | |||
There was a problem hiding this comment.
Explicitly map our SO attributes to the response
| @@ -10,9 +10,3 @@ | |||
| * Fetch Telemetry Config | |||
| */ | |||
| export const FetchTelemetryConfigRoute = '/api/telemetry/v2/config'; | |||
There was a problem hiding this comment.
This is kind of an interesting one... I am deferring changing this path as I am not familiar with the original context for introducing v2 in it, but I am guessing once an actual versioning mechanism is available we can come back to it? Let me know what y'all think.
There was a problem hiding this comment.
That is a very good question!! I need to rely on @Bamieh because the /v2/ part of the telemetry APIs predates me 🤷
FWIW, our telemetry APIs should be considered internal (we probably should name them as such, TBH)... Having said that, we are aware that some external products already use them, so I don't think we can introduce a breaking change without notifying them (or providing a sensible deprecation period).
There was a problem hiding this comment.
the /v2/ part of the telemetry APIs
From what I can recall, 'v2' deals with encryption
There was a problem hiding this comment.
The versioning is introduced for external consumers to check for and detect how to handle incoming requests (our telemetry endpoint for example).
v1 -> v2 was for encryption. This way the telemetry cluster endpoint will check the request URL and will handle the request differently based on the version.
| @@ -10,9 +10,3 @@ | |||
| * Fetch Telemetry Config | |||
| */ | |||
| export const FetchTelemetryConfigRoute = '/api/telemetry/v2/config'; | |||
There was a problem hiding this comment.
That is a very good question!! I need to rely on @Bamieh because the /v2/ part of the telemetry APIs predates me 🤷
FWIW, our telemetry APIs should be considered internal (we probably should name them as such, TBH)... Having said that, we are aware that some external products already use them, so I don't think we can introduce a breaking change without notifying them (or providing a sensible deprecation period).
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @jloleysens |
## Summary In this PR we ensure that we are adhering to the goals of versioned HTTP APIs to: 1. Not send SO attributes directly over the wire 2. Have a separate set of interfaces that can be referenced by public and maintained by server
Summary
In this PR we ensure that we are adhering to the goals of versioned HTTP APIs to: