fix: fix span logging based on changes to request types timestamps #12393
Conversation
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
Signed-off-by: Callum Styan <callumstyan@gmail.com>
cstyan
changed the title
There was a problem hiding this comment.
I understand the want for tests, but 800 lines of imports + mocks feels overkill to just solve timestamp.Time(r.GetStart()). I went and looked and this is exactly what it was prior to the pr which broke it
pkg/querier/queryrange/codec.go
Outdated
| @@ -249,8 +249,8 @@ func (r *LabelRequest) WithQuery(query string) queryrangebase.Request { | |||
|
|
|||
| func (r *LabelRequest) LogToSpan(sp opentracing.Span) { | |||
| sp.LogFields( | |||
| otlog.String("start", timestamp.Time(r.GetStart().UnixNano()).String()), | |||
| otlog.String("end", timestamp.Time(r.GetEnd().UnixNano()).String()), | |||
| otlog.String("start", timestamp.Time(r.Start.UnixNano()/(int64(time.Millisecond)/int64(time.Nanosecond))).String()), | |||
There was a problem hiding this comment.
What is going on here? This can just be timestamp.Time(r.GetStart()).
This is doing millis (r.GetStart) -> nanos (.UnixNano) -> divide by milli_duration -> divide by nano_duration (which is just "divide by 1")
There was a problem hiding this comment.
We can definitely simplify this, it just depends on what we want the time string in the span to look like.
otlog.String("start", timestamp.Time(r.GetStart().UnixMilli()).String()), would give us the exact format we had in the past, something like "2024-03-29 22:27:27.141 +0000 UTC", where as otlog.String("start", r.GetStart().String()), would give us something like "2024-03-29 22:27:27.141919586 +0000 UTC m=+1000.015209489".
For now I'll go with the former as it matches what we had before and is shorter.
Signed-off-by: Callum Styan <callumstyan@gmail.com>
commit 018856c Author: Callum Styan <callumstyan@gmail.com> Date: Mon Apr 1 06:40:16 2024 -0700 fix: fix span logging based on changes to request types timestamps (#12393) Signed-off-by: Callum Styan <callumstyan@gmail.com> commit 5190dda Author: Shantanu Alshi <shantanu.alshi@grafana.com> Date: Mon Apr 1 18:30:21 2024 +0530 feat(detected_labels): Initial skeleton for the API (#12390) Co-authored-by: Cyril Tovena <cyril.tovena@gmail.com> commit 0b7ff48 Author: Sandeep Sukhani <sandeep.d.sukhani@gmail.com> Date: Mon Apr 1 14:21:50 2024 +0530 chore: delete request processing improvements (#12259) commit a509871 Author: Ed Welch <edward.welch@grafana.com> Date: Sun Mar 31 22:14:21 2024 -0400 chore: remove experimental flags for l2 cache and memcached "addresses" config (#12410) commit 7480468 Author: Kaviraj Kanagaraj <kavirajkanagaraj@gmail.com> Date: Sun Mar 31 18:00:53 2024 +0200 fix: (Bug) correct resultType when storing instant query results in cache (#12312) Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com> commit 246623f Author: Trevor Whitney <trevorjwhitney@gmail.com> Date: Fri Mar 29 17:05:36 2024 -0600 fix(detected_fields): fix issues with frontend integration (#12406) This PRs fixes issues we found when integrating with the frontend * the `/experimental` api made it difficult to interact with using the existing datasource, so move to `v1/detected_fields` * the config flag was considered cumbersome as the only potential negative impact of the endpoint is when it is used, and nothing is currently using it * the use of an enum in the protobuf produced unexpected results in the json, so type was converted to string
…rafana#12393) Signed-off-by: Callum Styan <callumstyan@gmail.com>
In #11018 we modified various loki request types so that their
GetStart/GetEndfunctions returntime.Timerather thanint64. The underlying motivation was to allow the protobuf definition for request types to use actual time types instead of int64 types.However, in that PR we broke the span logging for some request types. In the first commit here, the tests for
TestLokiRequestSpanLogging,TestLokiSeriesRequestSpanLogging, andTestLabelRequestSpanLoggingwill fail. The second commit fixes the logging of the timestamps to the span.