Loki: Entry package

Created on 13 Dec 2019  Â·  7Comments  Â·  Source: grafana/loki

I was searching in our code base entry struct and find out many package has its own entry struct implementation.

I think we can create a new entry package where we would have the type share and tools for conversion and sorting.

componenloki good first issue help wanted keepalive kinenhancement

All 7 comments

@cyriltovena Would like to work on this. Can this be assigned to me ? I also need small brief

Sure, I can help with context. Here's a list of files that define similar entry structs

./cmd/fluent-bit/loki_test.go
12:type entry struct {
13-     lbs  model.LabelSet
14-     line string
15-     ts   time.Time
16-}

./pkg/loghttp/query.go
80:type Entry struct {
81-     Timestamp time.Time
82-     Line      string
83-}
84-

./pkg/chunkenc/memchunk.go
137:type entry struct {
138-    t int64
139-    s string
140-}
141-

./pkg/logproto/logproto.pb.go
416:type Entry struct {
417-    Timestamp time.Time `protobuf:"bytes,1,opt,name=timestamp,proto3,stdtime" json:"ts"`
418-    Line      string    `protobuf:"bytes,2,opt,name=line,proto3" json:"line"`
419-}
420-

./pkg/loghttp/legacy/query.go
19:type Entry struct {
20-     Timestamp time.Time `json:"ts"`
21-     Line      string    `json:"line"`
22-}

./pkg/promtail/api/entry_parser_test.go
17:type Entry struct {
18-     Time   time.Time
19-     Log    string
20-     Labels model.LabelSet
21-}

./pkg/promtail/client/client.go
99:type entry struct {
100-    tenantID string
101-    labels   model.LabelSet
102-    logproto.Entry
103-}

In order to avoid redundancies, type conversions, and a generally painful programming experience when crossing the borders between different definitions, we'll probably want to centralize definitions. This may be served by a model package. Perhaps something like https://github.com/prometheus/common/blob/master/model/time.go

@evalsocket - Are you still working on this? If not, I would like to pick it up. Let me know!

I’m guessing we’ll probably need a few variants, perhaps like this:

type Entry struct {
    Ts int64
    Line string
}

type Entries []Entry

type LabeledEntry struct {
    Entry
    Labels model.LabelSet
}

type LabeledEntries struct {
    Entries Entries
    Labels  model.LabelSet
}

This way we have a few structs for different use cases:
1) when we just have a log line
2) When we have a collection of log lines
3) when we have a log line with labels attached
4) when we have a collection of log lines that share labels

I'm unsure about using int64 vs time.Time for the Ts field.

Package name could be model as suggested by owen

This may be best fit for another issue, but I've experienced a lot of friction with some other type conversions. Examples include

  • logproto <> loghttp definitions
  • cortex client/label adapters <> our labels <> prometheus labels
  • promql.Result <> logql.Result <> loghttp.Result <> queryrange.Response (both cortex & loki)

These are just off the top of my head, but illustrate some pain pts.

I keep thinking we really need to refactor to remove type duplications everywhere:
1) Type differences confuse me b/c I need to investigate whether it’s done for a reason or is just a relic of convenience — I actually lose a lot of time this way
2) It’s super aggravating
3) We do a lot of O(n) conversions unnecessarily

/cc @slim-bean @cyriltovena

Closing this because it's extremely hard to get a consensus on the direction to take it, we aren't going to forget about it and i'm sure it will bet revisited some day but I don't see the value of keeping it open at the moment.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

pandey-adarsh147 picture pandey-adarsh147  Â·  4Comments

cyriltovena picture cyriltovena  Â·  4Comments

bzon picture bzon  Â·  5Comments

kylos101 picture kylos101  Â·  4Comments

negbie picture negbie  Â·  3Comments