Conversation
aa8c0c2 to
387feea
Compare
|
The buffering part of this PR seems like it's adding a lot of complexity and I'm not sure it's really the right solution anyway (it seems unlikely we'd ever exceed 8kb but if we did, shouldn't we just make a dedicate API call to cloud to report the information that couldn't be set in the header? I'd prefer we just rip out the buffering for now and add something to the header to indicate the payload was too big, and solve for that if we ever actually get to that point |
The buffer would also be used to cache telemetry reports for CLI calls that don't make API calls, such as |
10ac1b3 to
16cef38
Compare
internal/telemetry/transport.go
Outdated
| } | ||
|
|
||
| url := fmt.Sprintf("https://%s%s", s.host, telemetryEndpoint) | ||
| resp, err := http.Post(url, "application/json", bytes.NewReader(data)) |
There was a problem hiding this comment.
Should this be leveraging our other roundtripping logic s/t it picks up the access token if it exists?
There was a problem hiding this comment.
notably without a token, the WAF will probably block this
There was a problem hiding this comment.
Oh and CLI headers and such
There was a problem hiding this comment.
Good call - wrapped this in the existing roundtripper so we get tokens and user agent. Will tackle the WAF separately, but FWIW this PR should be safe to go in any time before that because it does not yet wire up any real calls to the telemetry endpoint.
There was a problem hiding this comment.
Logged RWX-65: Allow unauthenticated POST to /api/telemetry through WAF and marked the rest of the CLI issues in Linear as being blocked by that work.
Use the API client's RoundTripper for telemetry transport so requests pick up the access token, User-Agent header, and host configuration automatically.
16cef38 to
036d518
Compare
Summary
internal/telemetry/package with collector and senderPOST /api/telemetryFlush()fires a non-blocking goroutine to deliver queued eventsTest plan