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

move datanode to internal/node #82

Merged
merged 2 commits into from
Dec 3, 2024
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
31 changes: 15 additions & 16 deletions best_practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ A guide for getting the most out of Clues.
## CTX

The clues package can leverage golang's `context.Context` to pack in
metadata that you can later retrieve for logging or observation. These
metadata that you can later retrieve for logging or observation. These
additions form a tree, so you're always safe to extend or replace existing
state.

### Tracing

Calling `AddTrace` or `AddTraceName` will append to an internal property
with the key `clues_trace`. Clues traces form a slice of comma delimited
hashes (or names, when using TraceName). These can be useful for filtering
with the key `clues_trace`. Clues traces form a slice of comma delimited
hashes (or names, when using TraceName). These can be useful for filtering
logs to certain process branches.

clues.Add() always automatically appends to the trace, so you don't need
Expand Down Expand Up @@ -57,16 +57,15 @@ for _, user := range users {

Errors are the bottom-to-top metadata tracing counterpart to contexts.
At minimum, they replicate the creation, wrapping, and stacking of
errors. You can also label clues errors for broad categorization and
errors. You can also label clues errors for broad categorization and
add key:value metadata sets, including the full set of clues embedded
in a ctx value.


### Always Stack

A single-error Stack ensures clues will append a stacktrace reference
for that point of return. New(), Wrap() and Stack()ing multiple errors
all do this same process. This tip is specific for cases where you'd
for that point of return. New(), Wrap() and Stack()ing multiple errors
all do this same process. This tip is specific for cases where you'd
normally `return err`.

```go
Expand Down Expand Up @@ -122,8 +121,8 @@ if err != nil {
### Log the Core

The clues within errors aren't going to show up in a log message
or test output naturally. The formatted value will only include
the message. You can easily extract the full details using ToCore().
or test output naturally. The formatted value will only include
the message. You can easily extract the full details using ToCore().

```go
// in logs:
Expand All @@ -135,7 +134,7 @@ assert.NoError(t, err, clues.ToCore(err))
### Labels Are Not Sentinels

Clues errors already support Is and As, which means you can use them
for errors.Is and errors.As calls. This means you shouldn't use
for errors.Is and errors.As calls. This means you shouldn't use
labels for the same purpose.

```go
Expand All @@ -146,26 +145,26 @@ clues.Stack(errByRespCode(resp.StatusCode), err)
```

The best usage of labels is when you want to add _identifiable_ metadata
to an error. This is great for conditions where multiple different
errors can be flagged to get some specific handling. This allows you
to an error. This is great for conditions where multiple different
errors can be flagged to get some specific handling. This allows you
to identify the condition without altering the error itself.

```go
// example 1:
// example 1:
// doesn't matter what error took place, we want to end in a
// certain process state as a categorical result
for err := range processCh {
if clues.HasLabel(err, mustFailBackup) {
// set the backup state to 'failed'
// set the backup state to 'failed'
}
}

// example 2:
// we can categorically ignore errors based on configuration.
// we can categorically ignore errors based on configuration.
for _, err := range processFailures {
for _, cat := range config.ignoreErrorCategories {
if clues.HasLabel(err, cat) {
processFailures.IgnoreError(err)
processFailures.IgnoreError(err)
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions clog/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"reflect"

"github.com/alcionai/clues"
"github.com/alcionai/clues/cluerr"
"github.com/alcionai/clues/internal/node"
"github.com/alcionai/clues/internal/stringify"
otellog "go.opentelemetry.io/otel/log"
"go.uber.org/zap"
Expand Down Expand Up @@ -67,11 +69,11 @@ func (b builder) log(l logLevel, msg string) {

if b.err != nil {
// error values should override context values.
maps.Copy(cv, clues.InErr(b.err).Map())
maps.Copy(cv, cluerr.CluesIn(b.err).Map())

// attach the error and its labels
cv["error"] = b.err
cv["error_labels"] = clues.Labels(b.err)
cv["error_labels"] = cluerr.Labels(b.err)
}

// finally, make sure we attach the labels and comments
Expand All @@ -86,15 +88,15 @@ func (b builder) log(l logLevel, msg string) {
for k, v := range cv {
zsl = zsl.With(k, v)

attr := clues.NewAttribute(k, v)
attr := node.NewAttribute(k, v)
record.AddAttributes(attr.KV())
}

// plus any values added using builder.With()
for k, v := range b.with {
zsl.With(k, v)

attr := clues.NewAttribute(stringify.Fmt(k)[0], v)
attr := node.NewAttribute(stringify.Fmt(k)[0], v)
record.AddAttributes(attr.KV())
}

Expand Down
26 changes: 13 additions & 13 deletions clog/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"testing"

"github.com/alcionai/clues"
"github.com/alcionai/clues/cluerr"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -53,17 +53,17 @@ func TestBuilder(t *testing.T) {

bld.SkipCaller(1)

// ensure no collision between separate builders
// using the same ctx.
err := clues.New("an error").
With("fnords", "i have seen them").
Label("errLabel")
// ensure no collision between separate builders
// using the same ctx.
err := cluerr.New("an error").
With("fnords", "i have seen them").
Label("errLabel")

other := CtxErr(ctx, err)
assert.Empty(t, other.with)
assert.Empty(t, other.labels)
assert.Empty(t, other.comments)
assert.ErrorIs(t, other.err, err, clues.ToCore(err))
other := CtxErr(ctx, err)
assert.Empty(t, other.with)
assert.Empty(t, other.labels)
assert.Empty(t, other.comments)
assert.ErrorIs(t, other.err, err, cluerr.ToCore(err))

other.With("foo", "smarf")
assert.Contains(t, other.with, "foo")
Expand Down Expand Up @@ -126,8 +126,8 @@ func runErrorLogs(

func TestGetValue(t *testing.T) {
var (
p1 int = 1
ps string = "ptr"
p1 = 1
ps = "ptr"
pn any
)

Expand Down
21 changes: 21 additions & 0 deletions clog/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package clog

import (
"context"
"errors"
"os"
"sync"
"time"
Expand Down Expand Up @@ -223,6 +224,26 @@ func CtxErr(ctx context.Context, err error) *builder {
return nb
}

// Singleton is a shorthand for .Ctx(context.Background()). IE: it'll use the singleton
// logger directly; building one if necessary. You should avoid this and use .Ctx or
// .CtxErr if possible. Likelihood is that you're somewhere deep in a func chain that
// doesn't accept a ctx, and you still want to add a quick log; maybe for debugging purposes.
//
// That's fine! Everything should work great.
//
// Unless you call this before initialization. Then it'll panic. We do want you to init
// the logger first, else you'll potentially lose these logs due different buffers.
func Singleton() *builder {
if cloggerton == nil {
panic(errors.New("clog singleton requires prior initialization"))
}

return &builder{
ctx: context.Background(),
zsl: cloggerton.zsl,
}
}

// Flush writes out all buffered logs.
// Probably good to do before shutting down whatever instance
// had initialized the singleton.
Expand Down
8 changes: 3 additions & 5 deletions clog/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@ import (

"golang.org/x/exp/slices"

"github.com/alcionai/clues"
"github.com/alcionai/clues/cecrets"
"github.com/alcionai/clues/cluerr"
)

// ---------------------------------------------------
// consts
// ---------------------------------------------------

const clogLogFileEnv = "CLOG_LOG_FILE"

type logLevel string

const (
Expand Down Expand Up @@ -84,14 +82,14 @@ func (s Settings) LogToStdOut() Settings {
// LogToFile defines a system file to write all logs onto.
func (s Settings) LogToFile(pathToFile string) (Settings, error) {
if len(pathToFile) == 0 {
return s, clues.New("missing filepath for logging")
return s, cluerr.New("missing filepath for logging")
}

logdir := filepath.Dir(pathToFile)

err := os.MkdirAll(logdir, 0o755)
if err != nil {
return s, clues.Wrap(err, "ensuring log file dir exists").
return s, cluerr.Wrap(err, "ensuring log file dir exists").
With("log_dir", logdir)
}

Expand Down
4 changes: 2 additions & 2 deletions clog/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"path/filepath"
"testing"

"github.com/alcionai/clues"
"github.com/alcionai/clues/cluerr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -40,7 +40,7 @@ func TestSettings_LogToFile(t *testing.T) {
for _, test := range table {
t.Run(test.name, func(t *testing.T) {
set, err := Settings{}.LogToFile(test.input)
test.expectErr(t, err, clues.ToCore(err))
test.expectErr(t, err, cluerr.ToCore(err))
assert.Equal(t, test.expectOverride, set.fileOverride)
})
}
Expand Down
Loading
Loading