From 70c051770bd1b92ad20befc181bf7db2213a023a Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 14 Aug 2026 15:35:03 +0200 Subject: [PATCH] feat(telemetry): Go helper so the CLI can emit a conformant event (backend#1897) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC-BACKEND-1872 D2 for the CLI, and D12's host-process path. The Go half of what backend#1896 does for the Python services. Unblocks #1907. The CLI emits nothing today — it is not a pod, so the edge Collector's filelog receiver cannot reach it, and a field failure is only ever a support thread. e := telemetry.New(cfg.CurrentEnv, version, hostname) err := e.Emit("cli.command.failed", telemetry.Attrs{"error.type": "network"}) ENFORCED, NOT DOCUMENTED. The contract's mechanically-checkable rules run at the call site and return an error: the .. grammar with its closed vocabularies, the attribute-key namespace, retired names, value types, and the error set a failure must carry — stacktrace included. It returns rather than panics, because a CLI must never die of telemetry; but a malformed event must not pass silently either, and the caller's tests are where it fails. THE ENVIRONMENT IS DERIVED, NOT RESTATED. `New` classifies via `api.IsKnownEnv`, the same function that rejects a `--env staging` typo at the CLI's front door, and a test asserts the two agree across dev/stg/prod/staging/ prd/PROD/"". One saying yes while the other says no is precisely how records acquire a guessed environment. The domain vocabulary is narrower than the full registry — `cli` and `auth` only. The CLI is not the installer and not the backend, so admitting domains it cannot legitimately produce would make a typo look plausible. That is the failure already visible in the browser leg, where 461 of 484 events are named `not_specified`. 25 tests, 100% statement coverage, `make check` green. Twelve rules mutation-proved. ONE SURVIVED THE FIRST PASS, and it was a weak test rather than weak code — the same one the Python side hit. Every key I had tried was caught by the retired or namespace rule, so nothing exercised the key-SHAPE check; it needed a key that passes every other rule and is still badly shaped (`tracebloc.clientID`). Added, and the mutation now bites. NOTHING IMPORTS THIS YET, deliberately: #1907 is the consumer ticket. One consequence worth recording rather than discovering — `make deadcode` scans reachability from ./cmd/tracebloc, so a package outside that import graph is invisible to it. The gate is SILENT on this package, not passing it, and will start covering it the moment #1907 wires the first call site. Co-Authored-By: Claude Opus 5 --- internal/telemetry/telemetry.go | 297 +++++++++++++++++++++++++ internal/telemetry/telemetry_test.go | 312 +++++++++++++++++++++++++++ 2 files changed, 609 insertions(+) create mode 100644 internal/telemetry/telemetry.go create mode 100644 internal/telemetry/telemetry_test.go diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go new file mode 100644 index 0000000..7c518ed --- /dev/null +++ b/internal/telemetry/telemetry.go @@ -0,0 +1,297 @@ +// Package telemetry emits contract-conformant events from the CLI. +// +// RFC-BACKEND-1872 D2, backend#1897. The normative rules are +// rfcs/specs/backend-1872-telemetry-contract.md; this package is the Go half of +// what the shared Python emitter (backend#1896) does for the services. +// +// WHY THE CLI NEEDS ITS OWN. Every other Class A component is a pod, so the +// edge Collector's filelog receiver reaches its stdout. The CLI is not a pod +// and runs on a user's machine, so nothing collects it — it emits its own +// outcome events through the same gateway and token (D5). Today it emits +// nothing at all, which is why a field failure is only ever a support thread. +// +// WHAT IS ENFORCED. The contract's mechanically-checkable rules are checked +// here, at the call site, and an invalid event is an error the caller cannot +// ignore: the .. grammar with its closed vocabularies, +// the attribute-key namespace, retired names, and the error set a failure must +// carry. A CLI that reports a malformed event is worse than one that reports +// nothing, because the malformed one looks like coverage. +package telemetry + +import ( + "fmt" + "regexp" + "sort" + "strings" + + "github.com/tracebloc/cli/internal/api" +) + +// Service is this component's registry identity (contract §10.1). It is a +// constant, never derived from os.Args[0] or the binary's name — deriving +// service identity from the process is the defect the contract exists to close. +const ( + Service = "cli" + Component = "cli" +) + +// UnknownVersion is what a build that injected no version reports (§4). A +// value, not an omission: it is queryable and alertable, and an absent key is +// neither. `go build` without -ldflags produces "dev", which maps here. +const UnknownVersion = "0.0.0-unknown" + +// eventNameRe is §6.1's grammar: exactly three lowercase segments. +var eventNameRe = regexp.MustCompile(`^[a-z][a-z0-9_]*(\.[a-z][a-z0-9_]*){2}$`) + +// attrKeyRe is §1.1: lowercase, dot-separated, snake_case leaves. +var attrKeyRe = regexp.MustCompile(`^[a-z][a-z0-9_]*(\.[a-z][a-z0-9_]*)*$`) + +// domains the CLI may emit under (§6.3). Narrower than the full registry on +// purpose: the CLI is not the installer and not the backend, so admitting +// domains it cannot legitimately produce would make a typo look plausible. +var domains = map[string]bool{ + "cli": true, // command execution + "auth": true, // login, token refresh — §6.3 names the cli as a primary emitter +} + +// outcomes is §6.4, in full. Past tense throughout: an event records something +// that happened. +var outcomes = map[string]bool{ + "started": true, "succeeded": true, "failed": true, "skipped": true, + "rejected": true, "retried": true, "timed_out": true, "expired": true, + "cancelled": true, "completed": true, +} + +// failureOutcomes oblige the §8.4 error set. +var failureOutcomes = map[string]bool{ + "failed": true, "rejected": true, "timed_out": true, +} + +// retired names must not be emitted (§8.5). Each produced a measured defect and +// is replaced, not renamed. +var retired = map[string]bool{ + "env": true, "platform": true, "log_time": true, "traceback": true, + "extraData": true, "pod-name": true, "pod-status": true, + "experimentKey": true, "experimentId": true, +} + +// otelAttrs are the only keys allowed outside the tracebloc. namespace (§1.1). +var otelAttrs = map[string]bool{ + "service.name": true, "service.version": true, "service.instance.id": true, + "deployment.environment": true, "error.type": true, + "exception.type": true, "exception.message": true, "exception.stacktrace": true, + "event.name": true, +} + +// Attrs is one event's record attributes. Values are primitives (§1.2); a map +// or a serialised blob standing in for structure is the retired extraData +// defect, and is refused. +type Attrs map[string]any + +// Emitter holds the resource attributes — set once, at startup, never per call +// site (§1). A call site that could change them is the layering bug that made +// cloud_RoleName report the process name. +type Emitter struct { + resource map[string]string + sink func(map[string]string, map[string]any) +} + +// New builds the emitter for this process. +// +// env is the CLI's own current environment ("dev"/"stg"/"prod" — the same +// values internal/api declares and internal/config stores). version is the +// -ldflags-injected build version; "dev" or empty becomes UnknownVersion. +// +// An unrecognised env is NOT an error and NOT a guess: the emitter is built +// with exporting disabled, because a value no query filters on is worse than no +// record at all (§3.2). Ask Exports() when that distinction matters. +func New(env, version, instanceID string) *Emitter { + e := &Emitter{resource: map[string]string{ + "service.name": Service, + "tracebloc.component": Component, + "service.version": normaliseVersion(version), + "service.instance.id": instanceID, + }} + if api.IsKnownEnv(env) { + e.resource["deployment.environment"] = strings.ToLower(env) + } + return e +} + +func normaliseVersion(v string) string { + v = strings.TrimSpace(v) + // "dev" is what `go build` without -ldflags reports (see cmd/tracebloc). + // It is not a released artifact, so it is not a service.version. + if v == "" || v == "dev" { + return UnknownVersion + } + return v +} + +// Exports reports whether this process may send records to the hub. +// +// Only the three known backend environments export. There is no "local" or +// "ci" for the CLI the way there is for a service — a developer's machine +// running against dev IS dev — so the distinction here is known-vs-unknown, and +// unknown never exports. +func (e *Emitter) Exports() bool { + _, ok := e.resource["deployment.environment"] + return ok +} + +// Resource returns a copy of the per-process attributes. +func (e *Emitter) Resource() map[string]string { + out := make(map[string]string, len(e.resource)) + for k, v := range e.resource { + out[k] = v + } + return out +} + +// Emit validates one occurrence against the contract and hands it to the sink. +// +// It returns an error rather than panicking or logging: a CLI must never die +// because telemetry was malformed, but a malformed event must not pass silently +// either — the caller's tests are where this is meant to fail. +func (e *Emitter) Emit(eventName string, attrs Attrs) error { + if err := CheckEventName(eventName); err != nil { + return err + } + record, err := normalise(attrs) + if err != nil { + return err + } + if err := checkFailureSet(eventName, record); err != nil { + return err + } + record["event.name"] = eventName + if e.sink != nil { + e.sink(e.Resource(), record) + } + return nil +} + +// SetSink installs the delivery function. Nil means validate-and-drop, which is +// what an unconfigured or non-exporting process does — the contract checks +// still run, so a bad event fails in CI regardless of where the binary is. +func (e *Emitter) SetSink(f func(resource map[string]string, record map[string]any)) { + e.sink = f +} + +// CheckEventName applies §6.1's grammar and §6.3/§6.4's closed vocabularies. +func CheckEventName(name string) error { + if !eventNameRe.MatchString(name) { + return fmt.Errorf( + "telemetry: event.name %q must be .. (%s)", + name, eventNameRe.String()) + } + parts := strings.Split(name, ".") + if !domains[parts[0]] { + return fmt.Errorf( + "telemetry: event.name %q uses domain %q, which the CLI may not emit; known: %s", + name, parts[0], keys(domains)) + } + if !outcomes[parts[2]] { + return fmt.Errorf( + "telemetry: event.name %q ends in %q, which is not a registered outcome (known: %s). "+ + "If the verb you want is missing, the ACTION belongs in the object segment: "+ + "auth.token_refresh.succeeded, not auth.token.refreshed", + name, parts[2], keys(outcomes)) + } + return nil +} + +func normalise(attrs Attrs) (map[string]any, error) { + out := make(map[string]any, len(attrs)) + for key, value := range attrs { + // §1.2 — an absent value is omitted, never sent as nil or "". The + // retired `traceback` key rode on every record and was empty on 99.8%. + if value == nil { + continue + } + if s, ok := value.(string); ok && s == "" { + continue + } + if err := checkAttrKey(key); err != nil { + return nil, err + } + if err := checkAttrValue(key, value); err != nil { + return nil, err + } + out[key] = value + } + return out, nil +} + +func checkAttrKey(key string) error { + if retired[key] { + return fmt.Errorf( + "telemetry: attribute %q is retired (contract §8.5) and must not be emitted; "+ + "retired names are replaced, not renamed", key) + } + if otelAttrs[key] { + return nil + } + if !attrKeyRe.MatchString(key) { + return fmt.Errorf( + "telemetry: attribute key %q must be lowercase dot-separated with snake_case "+ + "leaves (%s) — not camelCase, not kebab-case", key, attrKeyRe.String()) + } + if !strings.HasPrefix(key, "tracebloc.") { + return fmt.Errorf( + "telemetry: attribute key %q is neither an OpenTelemetry attribute nor under "+ + "the tracebloc. namespace (contract §1.1)", key) + } + return nil +} + +func checkAttrValue(key string, value any) error { + switch value.(type) { + case string, bool, int, int64, float64: + return nil + default: + return fmt.Errorf( + "telemetry: attribute %q has type %T; values must be primitives (contract §1.2). "+ + "A map or a JSON string standing in for structure is the retired extraData "+ + "defect — nothing can query inside it", key, value) + } +} + +func checkFailureSet(eventName string, record map[string]any) error { + parts := strings.Split(eventName, ".") + if !failureOutcomes[parts[2]] { + return nil + } + if _, ok := record["error.type"]; !ok { + return fmt.Errorf( + "telemetry: %q is a failure and must carry error.type — a stable, "+ + "low-cardinality classification (contract §8.4). Without it a failure "+ + "cannot be grouped", eventName) + } + // §8.4 — where an exception was caught the stacktrace is REQUIRED, not + // optional. 0.2% of today's error records carry one. + caught := []string{"exception.type", "exception.message", "exception.stacktrace"} + var present, missing []string + for _, k := range caught { + if _, ok := record[k]; ok { + present = append(present, k) + } else { + missing = append(missing, k) + } + } + if len(present) > 0 && len(missing) > 0 { + return fmt.Errorf( + "telemetry: %q carries %v but not %v; the stacktrace is required where an "+ + "exception was caught (contract §8.4)", eventName, present, missing) + } + return nil +} + +func keys(m map[string]bool) string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return strings.Join(out, ", ") +} diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go new file mode 100644 index 0000000..0f2a890 --- /dev/null +++ b/internal/telemetry/telemetry_test.go @@ -0,0 +1,312 @@ +package telemetry + +import ( + "strings" + "testing" + + "github.com/tracebloc/cli/internal/api" +) + +// --- identity --------------------------------------------------------------- + +func TestServiceIdentityIsConstant(t *testing.T) { + // §2 — never derived from os.Args[0], the binary name, or a hostname. + // Deriving service identity from the process is what makes `gunicorn` a + // service in today's topology. + e := New(api.EnvProd, "0.10.7", "host-1") + r := e.Resource() + if r["service.name"] != "cli" || r["tracebloc.component"] != "cli" { + t.Fatalf("identity is not the registry value: %v", r) + } +} + +func TestEveryEnvironmentTheCLIDeclaresIsAccepted(t *testing.T) { + // DERIVED, not restated: these are internal/api's own constants. If the CLI + // gains an environment there, this fails until telemetry classifies it — + // the alternative is records landing under a value no query filters on. + for _, env := range []string{api.EnvDev, api.EnvStg, api.EnvProd} { + t.Run(env, func(t *testing.T) { + e := New(env, "0.10.7", "host-1") + if !e.Exports() { + t.Fatalf("%q is a declared CLI environment but does not export", env) + } + if got := e.Resource()["deployment.environment"]; got != env { + t.Fatalf("deployment.environment = %q, want %q", got, env) + } + }) + } +} + +func TestTelemetryAgreesWithIsKnownEnv(t *testing.T) { + // The two must not drift: api.IsKnownEnv is what rejects a `--env staging` + // typo at the CLI's front door, and this is what decides whether the same + // value reaches the hub. One saying yes while the other says no is how + // records get a guessed environment. + for _, env := range []string{"dev", "stg", "prod", "staging", "prd", "PROD", ""} { + t.Run(env, func(t *testing.T) { + e := New(env, "0.10.7", "host-1") + if e.Exports() != api.IsKnownEnv(env) { + t.Fatalf("Exports()=%v but api.IsKnownEnv(%q)=%v", + e.Exports(), env, api.IsKnownEnv(env)) + } + }) + } +} + +func TestAnUnknownEnvironmentIsNotGuessed(t *testing.T) { + // §3.2 — a value no query filters on is worse than no record. `staging` is + // the classic near miss: it is the git branch name, and `stg` is the + // environment value. + e := New("staging", "0.10.7", "host-1") + if e.Exports() { + t.Fatal("an unrecognised environment exported") + } + if _, ok := e.Resource()["deployment.environment"]; ok { + t.Fatal("an unrecognised environment was stamped anyway") + } +} + +func TestVersionUnknownIsAValueNotAnOmission(t *testing.T) { + // `go build` without -ldflags reports "dev", which is not a released + // artifact. §4 wants 0.0.0-unknown: queryable and alertable. + for _, in := range []string{"", "dev", " "} { + e := New(api.EnvProd, in, "host-1") + if got := e.Resource()["service.version"]; got != UnknownVersion { + t.Fatalf("version %q -> %q, want %q", in, got, UnknownVersion) + } + } + if got := New(api.EnvProd, "0.10.7", "h").Resource()["service.version"]; got != "0.10.7" { + t.Fatalf("an injected version was lost: %q", got) + } +} + +// --- event names ------------------------------------------------------------ + +func TestConformingEventNamesAreAccepted(t *testing.T) { + e := New(api.EnvProd, "0.10.7", "h") + for _, name := range []string{ + "cli.command.started", "cli.command.succeeded", + "auth.token_refresh.succeeded", "auth.login.failed", + } { + t.Run(name, func(t *testing.T) { + attrs := Attrs{} + if strings.HasSuffix(name, "failed") { + attrs["error.type"] = "x" + } + if err := e.Emit(name, attrs); err != nil { + t.Fatalf("rejected a conforming name: %v", err) + } + }) + } +} + +func TestTheGrammarIsExactlyThreeSegments(t *testing.T) { + for _, name := range []string{ + "cli.failed", "cli.command.run.failed", "CLI.command.failed", + "cli.Command.failed", "cli-command-failed", "cli..failed", "", + } { + t.Run(name, func(t *testing.T) { + if err := CheckEventName(name); err == nil { + t.Fatalf("accepted a malformed name %q", name) + } + }) + } +} + +func TestAnEventNameMayNotCarryASubject(t *testing.T) { + // `cli.command.e0qaz0zi.failed` has four segments — the grammar enforces + // "no subject in the name" rather than merely stating it. The subject is an + // attribute (§7). + if err := CheckEventName("cli.command.e0qaz0zi.failed"); err == nil { + t.Fatal("accepted a name carrying a subject") + } +} + +func TestADomainTheCLICannotEmitIsRefused(t *testing.T) { + // Narrower than the full registry on purpose: the CLI is not the installer + // and not the backend. 461 of 484 browser events are named `not_specified` + // because an unregistered value became a new silent namespace. + for _, name := range []string{ + "install.preflight.failed", "training.job.failed", "telemetry.thing.failed", + } { + if err := CheckEventName(name); err == nil { + t.Fatalf("accepted a domain the CLI may not emit: %q", name) + } + } +} + +func TestAnUnregisteredOutcomeIsRefusedAndSuggestsTheFix(t *testing.T) { + err := CheckEventName("auth.token.refreshed") + if err == nil { + t.Fatal("accepted an unregistered outcome") + } + if !strings.Contains(err.Error(), "auth.token_refresh.succeeded") { + t.Fatalf("the error does not point at the fix: %v", err) + } +} + +// --- attributes ------------------------------------------------------------- + +func TestRetiredNamesAreRefusedAsRetired(t *testing.T) { + // Asserting WHICH rule fires. Every retired name is also caught by the + // namespace or key-shape rule, so a test that only checks "it errors" + // passes with the retired check deleted — the Python side proved that under + // mutation. The message is what tells a caller the field was replaced. + e := New(api.EnvProd, "0.10.7", "h") + for name := range retired { + t.Run(name, func(t *testing.T) { + err := e.Emit("cli.command.succeeded", Attrs{name: "x"}) + if err == nil { + t.Fatalf("accepted retired attribute %q", name) + } + if !strings.Contains(err.Error(), "retired") { + t.Fatalf("%q was rejected by another rule, so the retired check "+ + "is doing no work: %v", name, err) + } + }) + } +} + +func TestKeyShapeAndNamespace(t *testing.T) { + e := New(api.EnvProd, "0.10.7", "h") + for _, key := range []string{"experimentKey", "pod-name", "cluster.name", "Foo"} { + t.Run(key, func(t *testing.T) { + if err := e.Emit("cli.command.succeeded", Attrs{key: "x"}); err == nil { + t.Fatalf("accepted key %q", key) + } + }) + } + // The shape rule needs a key that passes every OTHER rule: not retired, not + // an OTel name, correctly under `tracebloc.` — and badly shaped. Without + // one, the namespace rule catches every case and the shape check does no + // work; it survived mutation until this was added. + for _, key := range []string{ + "tracebloc.clientID", // camelCase leaf + "tracebloc.client-id", // kebab leaf + "tracebloc.Client.id", // capitalised segment + } { + t.Run(key, func(t *testing.T) { + err := e.Emit("cli.command.succeeded", Attrs{key: "x"}) + if err == nil { + t.Fatalf("accepted badly-shaped key %q", key) + } + if !strings.Contains(err.Error(), "snake_case") { + t.Fatalf("%q was rejected by another rule, so the key-shape "+ + "check is doing no work: %v", key, err) + } + }) + } + + if err := e.Emit("cli.command.succeeded", Attrs{"tracebloc.client.id": "abc"}); err != nil { + t.Fatalf("rejected a valid tracebloc key: %v", err) + } + if err := e.Emit("cli.command.succeeded", Attrs{"error.type": "x"}); err != nil { + t.Fatalf("rejected a valid OTel key: %v", err) + } +} + +func TestAbsentValuesAreOmittedNotSent(t *testing.T) { + // The retired `traceback` key rode on every record and was empty on 99.8%. + e := New(api.EnvProd, "0.10.7", "h") + var got map[string]any + e.SetSink(func(_ map[string]string, record map[string]any) { got = record }) + if err := e.Emit("cli.command.succeeded", Attrs{ + "tracebloc.client.id": nil, + "tracebloc.note": "", + }); err != nil { + t.Fatal(err) + } + for _, k := range []string{"tracebloc.client.id", "tracebloc.note"} { + if _, ok := got[k]; ok { + t.Fatalf("%q was sent empty rather than omitted", k) + } + } +} + +func TestNonPrimitiveValuesAreRefused(t *testing.T) { + e := New(api.EnvProd, "0.10.7", "h") + if err := e.Emit("cli.command.succeeded", Attrs{ + "tracebloc.detail": map[string]string{"a": "b"}, + }); err == nil { + t.Fatal("accepted a map value — that is the retired extraData defect") + } +} + +// --- failures --------------------------------------------------------------- + +func TestAFailureMustCarryErrorType(t *testing.T) { + e := New(api.EnvProd, "0.10.7", "h") + for _, outcome := range []string{"failed", "rejected", "timed_out"} { + t.Run(outcome, func(t *testing.T) { + if err := e.Emit("cli.command."+outcome, Attrs{}); err == nil { + t.Fatalf("accepted a %s with no error.type", outcome) + } + }) + } +} + +func TestACaughtExceptionMustBringItsStacktrace(t *testing.T) { + e := New(api.EnvProd, "0.10.7", "h") + err := e.Emit("cli.command.failed", Attrs{ + "error.type": "network", + "exception.type": "net.OpError", + "exception.message": "boom", + }) + if err == nil || !strings.Contains(err.Error(), "stacktrace") { + t.Fatalf("a partial exception set was accepted: %v", err) + } +} + +func TestACompleteFailureRecordIsAccepted(t *testing.T) { + e := New(api.EnvProd, "0.10.7", "h") + if err := e.Emit("cli.command.failed", Attrs{ + "error.type": "network", + "exception.type": "net.OpError", + "exception.message": "boom", + "exception.stacktrace": "goroutine 1…", + }); err != nil { + t.Fatalf("rejected a complete failure record: %v", err) + } +} + +// --- delivery --------------------------------------------------------------- + +func TestValidationRunsEvenWithNoSink(t *testing.T) { + // A non-exporting build must still fail a malformed event, or the contract + // is only enforced where it is least likely to be tested. + e := New("staging", "0.10.7", "h") // not exporting + if err := e.Emit("cli.command.refreshed", Attrs{}); err == nil { + t.Fatal("a non-exporting emitter skipped validation") + } +} + +func TestTheSinkReceivesResourceAndRecordSeparately(t *testing.T) { + // §1's layering: resource describes the emitter, record the occurrence. A + // sink that saw them merged could not tell which layer a key came from — + // which is the confusion that produced cloud_RoleName. + e := New(api.EnvProd, "0.10.7", "h") + var res map[string]string + var rec map[string]any + e.SetSink(func(r map[string]string, d map[string]any) { res, rec = r, d }) + if err := e.Emit("cli.command.succeeded", Attrs{"tracebloc.client.id": "abc"}); err != nil { + t.Fatal(err) + } + if res["service.name"] != "cli" { + t.Fatalf("resource lost its identity: %v", res) + } + if rec["event.name"] != "cli.command.succeeded" || rec["tracebloc.client.id"] != "abc" { + t.Fatalf("record is wrong: %v", rec) + } + if _, leaked := rec["service.name"]; leaked { + t.Fatal("a resource attribute leaked into the record layer") + } +} + +func TestResourceIsACopy(t *testing.T) { + // A call site that could mutate the resource is the §1 bug. + e := New(api.EnvProd, "0.10.7", "h") + e.Resource()["service.name"] = "backend" + if e.Resource()["service.name"] != "cli" { + t.Fatal("the resource is mutable from outside") + } +}