Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Logs UX #208

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions api/log/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import (
)

type filteredOutput struct {
out output.LogOutput
labels map[string]struct{}
out output.LogOutput
labels map[string]struct{}
lineCount int
}

func (o *filteredOutput) FormatAndPrintln(ts time.Time, lbls loghttp.LabelSet, maxLabelsLen int, line string) {
Expand All @@ -22,14 +23,29 @@ func (o *filteredOutput) FormatAndPrintln(ts time.Time, lbls loghttp.LabelSet, m
}
}
o.out.FormatAndPrintln(ts, lbls, maxLabelsLen, line)
o.lineCount++
}

func (o filteredOutput) WithWriter(w io.Writer) output.LogOutput {
return o.out.WithWriter(w)
}

func NewStdOut(mode string, noLabels bool, labels ...string) (output.LogOutput, error) {
out, err := output.NewLogOutput(os.Stdout, mode, &output.LogOutputOptions{
func (o filteredOutput) LineCount() int {
return o.lineCount
}

type Output interface {
output.LogOutput
// LineCount returns the amount of lines the output has processed
LineCount() int
}

func NewStdOut(mode string, noLabels bool, labels ...string) (Output, error) {
return NewOutput(os.Stdout, mode, noLabels, labels...)
}

func NewOutput(w io.Writer, mode string, noLabels bool, labels ...string) (Output, error) {
out, err := output.NewLogOutput(w, mode, &output.LogOutputOptions{
NoLabels: noLabels, ColoredOutput: true, Timezone: time.Local,
})
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions logs/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package logs
import (
"context"
"errors"
"fmt"
"time"

apps "github.com/ninech/apis/apps/v1alpha1"
Expand All @@ -24,6 +25,12 @@ func (cmd *buildCmd) Run(ctx context.Context, client *api.Client) error {
if err := client.Get(ctx, api.NamespacedName(cmd.Name, client.Project), build); err != nil {
return err
}
if time.Since(build.CreationTimestamp.Time) > logRetention {
return fmt.Errorf(
"the logs of the build %s are not available as the build is more than %.f days old",
build.Name, logRetention.Hours()/24,
)
}
cmd.Since = time.Since(build.CreationTimestamp.Time)
}

Expand Down
44 changes: 38 additions & 6 deletions logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"strings"
"time"

"github.com/grafana/loki/pkg/logcli/output"
"github.com/alecthomas/kong"
"github.com/grafana/loki/pkg/logproto"
"github.com/ninech/nctl/api"
"github.com/ninech/nctl/api/log"
Expand All @@ -24,18 +24,36 @@ type resourceCmd struct {
type logsCmd struct {
Follow bool `help:"Follow the logs by live tailing." short:"f"`
Lines int `help:"Amount of lines to output" default:"50" short:"l"`
Since time.Duration `help:"Duration how long to look back for logs" short:"s" default:"24h"`
Since time.Duration `help:"Duration how long to look back for logs" short:"s" default:"${log_retention}"`
From time.Time `help:"Ignore since flag and start looking for logs at this absolute time (RFC3339)" placeholder:"2025-01-01T14:00:00+01:00"`
To time.Time `help:"Ignore since flag and stop looking for logs at this absolute time (RFC3339)" placeholder:"2025-01-01T15:00:00+01:00"`
Output string `help:"Configures the log output format. ${enum}" short:"o" enum:"default,json" default:"default"`
NoLabels bool `help:"disable labels in log output"`
out output.LogOutput
out log.Output
}

// 30 days, we hardcode this for now as it's not possible to customize this on
// deplo.io. We'll need to revisit this if we ever make this configurable.
var logRetention = time.Duration(time.Hour * 24 * 30)

func (cmd *logsCmd) Run(ctx context.Context, client *api.Client, queryString string, labels ...string) error {
now := time.Now()
start, end := now.Add(-cmd.Since), now
if !cmd.From.IsZero() {
start = cmd.From
}
if !cmd.To.IsZero() {
end = cmd.To
}
if now.Sub(start) > logRetention {
return fmt.Errorf("the logs requested exceed the retention period of %.f days", logRetention.Hours()/24)
}

query := log.Query{
QueryString: queryString,
Limit: cmd.Lines,
Start: time.Now().Add(-cmd.Since),
End: time.Now(),
Start: start,
End: end,
Direction: logproto.BACKWARD,
Quiet: true,
}
Expand All @@ -53,7 +71,14 @@ func (cmd *logsCmd) Run(ctx context.Context, client *api.Client, queryString str
return client.Log.TailQuery(ctx, 0, out, query)
}

return client.Log.QueryRange(ctx, out, query)
if err := client.Log.QueryRange(ctx, out, query); err != nil {
return err
}
if out.LineCount() == 0 {
return fmt.Errorf("no logs found between %s and %s", start.Format(time.RFC3339), end.Format(time.RFC3339))
}

return nil
}

type queryOperator string
Expand All @@ -74,3 +99,10 @@ func buildQuery(expr ...string) string {
func inProject(project string) string {
return queryExpr(opEquals, "namespace", project)
}

// KongVars returns all variables which are used in the logs commands
func KongVars() kong.Vars {
result := make(kong.Vars)
result["log_retention"] = logRetention.String()
return result
}
46 changes: 37 additions & 9 deletions logs/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ import (
"testing"
"time"

"github.com/grafana/loki/pkg/logcli/output"
apps "github.com/ninech/apis/apps/v1alpha1"
"github.com/ninech/nctl/api"
"github.com/ninech/nctl/api/log"
"github.com/ninech/nctl/internal/test"
"github.com/stretchr/testify/assert"
)

Expand All @@ -32,8 +30,9 @@ func TestRun(t *testing.T) {
ctx := context.Background()

cases := map[string]struct {
cmd logsCmd
expectedLines int
cmd logsCmd
expectedLines int
expectedErrContains string
}{
"line limit": {
cmd: logsCmd{
Expand Down Expand Up @@ -65,27 +64,56 @@ func TestRun(t *testing.T) {
},
expectedLines: len(lines),
},
"exceeds retention": {
cmd: logsCmd{
Output: "default",
Lines: len(lines),
Since: logRetention * 2,
},
expectedErrContains: "the logs requested exceed the retention period",
},
"from/to flags override since": {
cmd: logsCmd{
Output: "default",
Lines: len(lines),
Since: logRetention * 2,
From: time.Now().Add(-time.Hour),
To: time.Now(),
},
expectedLines: len(lines),
},
"from flag alone overrides since": {
cmd: logsCmd{
Output: "default",
Lines: len(lines),
Since: logRetention * 2,
From: time.Now().Add(-time.Hour),
},
expectedLines: len(lines),
},
}

for name, tc := range cases {
tc := tc
t.Run(name, func(t *testing.T) {
var buf bytes.Buffer
out, err := output.NewLogOutput(&buf, log.Mode(tc.cmd.Output), &output.LogOutputOptions{
NoLabels: true, ColoredOutput: false, Timezone: time.Local,
})
out, err := log.NewOutput(&buf, log.Mode(tc.cmd.Output), true)
if err != nil {
t.Fatal(err)
}

tc.cmd.out = out

if err := tc.cmd.Run(ctx, apiClient, ApplicationQuery("app-name", "app-ns")); err != nil {
t.Fatal(err)
if tc.expectedErrContains != "" {
assert.ErrorContains(t, err, tc.expectedErrContains)
} else {
assert.NoError(t, err)
}
}

if tc.expectedLines != 0 {
assert.Equal(t, test.CountLines(buf.String()), tc.expectedLines)
assert.Equal(t, out.LineCount(), tc.expectedLines)
}

if tc.cmd.Output == "json" {
Expand Down
4 changes: 2 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type flags struct {
APICluster string `help:"Context name of the API cluster." default:"${api_cluster}" env:"NCTL_API_CLUSTER" hidden:""`
LogAPIAddress string `help:"Address of the deplo.io logging API server." default:"https://logs.deplo.io" env:"NCTL_LOG_ADDR" hidden:""`
LogAPIInsecure bool `help:"Don't verify TLS connection to the logging API server." hidden:"" default:"false" env:"NCTL_LOG_INSECURE"`
Verbose bool `help:"Show verbose messages."`
Verbose bool `help:"Show verbose messages."`
Version kong.VersionFlag `name:"version" help:"Print version information and quit."`
}

Expand Down Expand Up @@ -193,7 +193,7 @@ func kongVariables() (kong.Vars, error) {
if err != nil {
return nil, fmt.Errorf("error on application create kong vars: %w", err)
}
if err := merge(result, appCreateKongVars, create.MySQLKongVars(), create.PostgresKongVars()); err != nil {
if err := merge(result, appCreateKongVars, create.MySQLKongVars(), create.PostgresKongVars(), logs.KongVars()); err != nil {
return nil, fmt.Errorf("error when merging kong variables: %w", err)
}

Expand Down