grafana / loki

Like Prometheus, but for logs.
https://grafana.com/loki
GNU Affero General Public License v3.0
22.73k stars 3.31k forks source link

chore: Make dedicated iterator package #13273

Closed chaudum closed 12 hours ago

chaudum commented 1 week ago

What this PR does / why we need it:

Iterators of various types are widely used throughout the Loki code base. With the recent code additions of the bloom filters, a new set of utility functions for iterators emerged in github.com/grafana/loki/v3/pkg/storage/bloom/v1. The package defines interfaces for various common types, but also provides implementations to create and compose new iterators that implement these interfaces. This new package uses Go Generics.

The basic iterator interface (defined in pkg/iter/v2/interface.go) looks like so:

// Usage:
//
//  for it.Next() {
//      curr := it.At()
//      // do something
//  }
//  if it.Err() != nil {
//      // do something
//  }
type Iterator[T any] interface {
    Next() bool
    Err() error
    At() T
}

Discussion

I proposed to use the Err() function for getting the optional error, because it was used in the pkg/storage/bloom/v1 package. However, in existing iterators for entries and samples, this function was named Error(). I don't have strong arguments for or against one or the other name.

Next steps

This PR intentionally leaves the duplicate implementation of various iterator types, such a heap sort iterators, slice iterators, or peeking iterators. These should be consolidated in a follow up PR.

Checklist

grobinson-grafana commented 1 week ago

Sounds great to me! ❤️ I think let's use Err() as it seems idiomatic (see context.Context).