starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-20 16:02:19 +00:00
Code Issues Packages Projects Releases Wiki Activity
13,594 Commits 730 Branches 1,510 Tags
script-editor-edit-code-debug
Commit Graph

4 Commits

Author SHA1 Message Date
hugocasa fad1a549d9 feat(otel): connect jobs to the inbound distributed trace (#9456) 
* feat(otel): propagate inbound W3C traceparent to job spans

Capture the inbound traceparent header at the run endpoints
(WebhookArgs::to_args_from_format) into a reserved _wm_traceparent arg key
(gated on OTEL_TRACING_ENABLED), riding the args jsonb like
_ENTRYPOINT_OVERRIDE. At pickup, create_span_with_name attaches a span link
from the job's worker span to the originating distributed trace, so a job
triggered by an instrumented service is connected to the caller's trace
while keeping its UUID-derived trace id (trace-by-job-id unaffected).

The link/parse logic lives in the EE otel modules; this OSS side only
captures the header and calls the (no-op outside EE) hook. Companion EE PR
required.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: bump ee-repo-ref to inbound-trace-propagation EE branch

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(agents): don't attribute work to specific customers in repo content

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(otel): relocate job + script spans into the inbound trace

Builds on the captured _wm_traceparent: the worker job span is re-parented on
the inbound caller context, the script subprocess's TRACEPARENT env is the
inbound context (so its spans join the caller's trace), and the context is
propagated to flow steps so the whole flow relocates. Carried to the worker via
a new LogContext.inbound_traceparent field. Non-inbound jobs are unchanged.

Adds a relocation integration test. Companion EE PR required.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: bump ee-repo-ref to inbound-trace-propagation relocate commit

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(otel): harden inbound traceparent capture

Address review feedback:
- strip any caller-supplied _wm_traceparent from args/extra before stashing the
  header-captured value, so the reserved key is Windmill-controlled only
- valid_w3c_traceparent: reject version ff and require lowercase hex, so we don't
  forward an inbound header that downstream OTel parsers would reject
- clarify that the capture helper does not validate the W3C format (done at use)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to 2c7964460327fab5e3a27c0f74b8d6f26ab7f79a

This commit updates the EE repository reference after PR #604 was merged in windmill-ee-private.

Previous ee-repo-ref: 8fc04fb105dc49769205f7174d551a0d134d1bec

New ee-repo-ref: 2c7964460327fab5e3a27c0f74b8d6f26ab7f79a

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
 2026-06-05 16:50:07 +00:00
Ruben Fiszel aedf369174 fix: pair PG arg type with actual Rust binding to keep query_typed_raw safe (#8999) 
* fix: pair PG arg type with actual Rust binding to keep query_typed_raw safe

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(pg): wrap encoder errors with arg context, add fallback test

Followups on #8999 review:

- Wrap rust-postgres "error serializing parameter N" failures with the arg
  name, JSON value kind, and asserted Postgres type plus a hint about an
  explicit cast — so users see actionable context instead of an opaque
  WrongType.
- Drift-prevention meta-test: assert otyp_to_pg_type and convert_val agree
  on the Type for every recognised arg_t when the JSON value matches its
  natural Rust kind. Catches future drift if either side changes.
- Integration test for the prepare + query_raw fallback path: confirms
  unrecognised arg_t (custom enum) is routed through prepare and the
  server-resolved type appears in the failure surface — flips into a
  test failure if a regression accidentally routes unrecognised types
  through query_typed_raw.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg): add otyp_inferred flag + regex-based placeholder renumbering

Two follow-ups from the review of #8999:

1. **Issue #1 (Number/Bool + explicit text decl in WHERE)**

   Add `Arg::otyp_inferred: bool` to the parser. The PG SQL parser sets
   it `true` only at the "no info → fall back to text" site (bare `$N`,
   no inline cast, no `-- $N (TYPE)` decl). All other arg sources keep
   it `false`.

   In `convert_val` this flag distinguishes:
   - explicit text-like target (`-- $1 (text)` or `$1::text`) — coerce
     `Bool`/`Number` → `Box<String>` so `WHERE text_col = $1` works
     (`text = text` operator). Pre-#8988 behaviour, restored.
   - parser-default text (bare `$N`) — bind the value's natural Rust
     type so the regression case (`Value::Bool` against a real `bool`
     column via `CAST AS bool`) keeps working.

   `Arg` is in `windmill-parser`; the new field has `#[serde(default)]`
   so persisted signatures stay backward-compatible.

2. **Issue #4 ($5/$50 substring rewrite collision)**

   Replace the per-index `String::replace` chain (which turned `$50`
   into `$10` when oidx=5 was processed first) with a single regex
   pass. `\d+` is greedy, so `$5` and `$50` match as distinct units;
   indices outside the mapping are left intact.

3. Tests:
   - parser: `test_parse_pgsql_otyp_inferred_flag` covers bare/inline-
     cast/decl/mixed shapes.
   - executor unit: `convert_val_bool_against_every_arg_t` and
     `convert_val_*_number_*` split each text-like target into explicit
     vs inferred expectations.
   - executor unit: `renumber_sparse_placeholders_no_collision`.
   - integration: `test_postgresql_arg_type_combinations` adds 4 cases
     covering decl(text)+Number/Bool in WHERE, bare $1+Bool, and
     sparse positional args ($5/$50).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg+sdk): enum support, extended String arms, position-aware $N rewrite, SDK quality

Backend:

1. **`AnyTextValue` ToSql/FromSql wrapper**: vanilla `tokio_postgres`'s
   `ToSql for String` / `FromSql for String` reject `Kind::Enum` and
   `Kind::Domain` even though the wire format is plain UTF-8. The wrapper
   accepts those kinds in both directions. End result: explicit
   `$1::my_enum` / `CAST($1 AS my_enum)` casts now round-trip without the
   ugly `CAST($1::text AS my_enum)` workaround, AND `SELECT enum_col`
   results come back as JSON strings instead of erroring at the FromSql
   layer.

2. **#10 — Value::String → numeric/real/double/oid/bool**. Without these
   arms, a string-encoded value (`"3.14"`, `"true"`) for a non-text /
   non-temporal arg_t fell through to `Box<String> + TEXT`, which then
   failed at the server (no implicit cast text→numeric in expression
   context). Now strings are parsed into the matching native type with
   clear error messages on parse failure.

3. **Position-aware `$N` rewrite**: replaces the regex-based renumbering
   (which fixed the `$5/$50` substring collision but still walked through
   string literals and comments, mangling `'price: $5'` etc.) with a
   walk over `parse_pg_statement_arg_positions` — the same
   string/comment/dollar-quote-aware tokenizer used for index discovery.
   Adds `parse_pg_statement_arg_positions` to the parser's public API.

SDK:

4. **BigInt support**: `JSON.stringify(BigInt)` throws. The SDK now
   stringifies bigints before serialisation; the executor accepts
   numeric strings into BIGINT arg slots via the existing
   `Value::String → INT8` parsing arm. SDK-side `inferSqlType` is split
   so `BigInt` always resolves to `BIGINT` (was reaching
   `Number.isInteger(BigInt)` which returns false → wrong default).

5. **Homogeneous array auto-tag**: `${[1,2,3]}` against an `int[]` column
   now emits `$1::BIGINT[]` instead of `$1::JSON`. Detection covers
   primitive types only (number / bigint / string / boolean); mixed or
   nested arrays still fall back to JSON. Mixed int/float widens to
   `DOUBLE PRECISION[]`.

6. **`.query()` positional bug**: previously the `.query()` method
   abused the template-tag builder, which appended `$N::TYPE` after the
   user's literal SQL string instead of binding by position
   (`SELECT $1, $2` became `SELECT $1, $2$1::BIGINT`). Now `.query()`
   builds the executor-shaped content directly: a `-- $N argN (TYPE)`
   declaration block followed by the user's SQL verbatim.

Tests:

- Parser: `test_parse_pg_statement_arg_positions_skips_strings_and_comments`
  asserts string literals, comments, and dollar-quoted blocks don't
  produce positions (so renumbering doesn't mangle them).
- Executor unit: `renumber_sparse_placeholders_no_collision_no_string_mangling`
  uses the new position-aware path and includes string-literal + comment
  + `$$…$$` cases. Existing convert_val tests grow to cover new
  String→numeric/real/double/oid/bool arms.
- Integration: `test_postgresql_arg_type_combinations` adds 13 cases
  (enum round-trip both directions, string→numeric/real/double/bool/oid,
  string-literal `$N` non-mangling). The prepare-fallback test now
  asserts SUCCESS (not failure) for enum encoding via AnyTextValue.
- SDK: new `typescript-client/tests/sqlUtils.test.ts` (42 tests)
  exhaustively covering inferSqlType primitives + arrays,
  parseTypeAnnotation, datatable() template tag (with all the new
  shapes — BigInt, homogeneous arrays, RawSql, schema preamble),
  datatable().query() positional, and ducklake() shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg): replace DISCARD ALL with curated reset (preserves typeinfo cache)

Found while exhaustively probing custom-type DX: every cached-connection
reuse was running `DISCARD ALL`, whose included `DEALLOCATE ALL`
deallocates *all* prepared statements server-side — including the typeinfo
statements that tokio_postgres caches per-Client to resolve custom enum /
domain Oids. tokio_postgres still held `Statement` objects whose names
the server had forgotten, so the next custom-type query failed with
intermittent "prepared statement \"sN\" does not exist" errors. The
failure was easy to reproduce: any sequence that forced typeinfo lookup
for two different custom-type kinds on the same cached connection (e.g.
enum followed by domain) would hit it.

Replace `DISCARD ALL` with a curated reset that explicitly targets the
state we actually care about, *without* touching prepared statements:

  RESET ALL                     — GUC parameters (search_path, application
                                  _name, statement_timeout, …)
  RESET SESSION AUTHORIZATION   — undoes both `SET SESSION AUTHORIZATION`
                                  and `SET ROLE` (RESET ALL does NOT —
                                  these aren't GUC parameters, so without
                                  this an elevated role from a previous
                                  job would silently leak)
  UNLISTEN *                    — drops LISTEN registrations
  CLOSE ALL                     — closes open cursors

Trade-off: temp tables, advisory locks (session-scoped), and user-created
PREPARE statements may persist across cached-connection reuse — rare in
datatable / PG-script workloads. tokio_postgres's typeinfo cache survives
intact, so custom enum / domain queries are fast on subsequent reuse.

Tests:
- `test_postgresql_custom_types_on_cached_connection` — runs 10×
  alternating enum + domain queries on a cached connection. Pre-fix this
  failed with `prepared statement "sN" does not exist` after the first
  reuse; post-fix passes.
- `test_postgresql_set_role_does_not_leak_across_cached_connection` —
  switches `SET ROLE` and `SET SESSION AUTHORIZATION` to a non-postgres
  role, then runs a follow-up job and asserts current_user/session_user
  are restored. Specifically catches the case where someone might switch
  back to `RESET ALL` alone (which doesn't cover SET ROLE / SESSION
  AUTHORIZATION) and silently introduce a permission-leak vector.
- All existing session-isolation tests
  (`test_postgresql_cached_connection_resets_session`,
   `test_postgresql_single_worker_session_isolation`,
   `test_postgresql_100_jobs_cached`) continue to pass.

Found via end-to-end probing of datatable / PG-script DX, not previously
covered: the existing isolation tests only did `SET ROLE postgres`, the
connecting user, so the leak was invisible.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg): address PR #8999 review (cubic + claude)

cubic (P1, real bug):
- `convert_vec_val` for `timetz` array asserted `Type::TIMETZ_ARRAY`, but
  chrono `NaiveTime` only encodes for TIME (same caveat as the scalar
  arm). Switch to `Type::TIME_ARRAY`; rely on PG's implicit `time→timetz`
  assignment cast at the column site. Add an explicit unit test.

claude (#1, silent failure → explicit error):
- `Bool` + explicit `(char)` / `(character)` decl previously silently
  bound BOOL, hoping the server would cast at the use site — but PG has
  no implicit `bool→char` and the resulting error
  ("operator does not exist: bool = char") was opaque. Now error at
  bind time with an actionable hint to use `bool` decl or pass the
  value as a "t"/"f" string.

claude (#2, asymmetry doc):
- Object/Array still coerce to text on `matches!(typ, Typ::Str(_))`
  (covers both explicit AND inferred-default text), unlike Bool/Number
  which key on `explicit_text_target`. The asymmetry is intentional
  (no implicit `jsonb → text` cast in expression context vs PG having
  implicit `bool/int → text` casts) — added a body comment so future
  maintainers don't try to "align" them.

claude (#3, perf):
- `parse_pg_statement_arg_indices` and `parse_pg_statement_arg_positions`
  walked the SQL tokenizer twice. Fold into a single pass that derives
  the index set from the position list.

claude (#4, fmt drift):
- `cargo fmt` over the parser crates I touched with perl scripts in the
  earlier commit (windmill-parser-{sql,bash,ts,go,php,java,csharp,nu,py,
  rust,graphql,yaml,r}). Net cosmetic.

claude (#5, parseTypeAnnotation):
- One-line caveat in the SDK's `parseTypeAnnotation` that the returned
  string is presence-only (e.g. `${x}::DOUBLE PRECISION` returns
  `"DOUBLE"`, `CAST(${x} AS int)` returns `"int)"` — neither matches a
  real PG type, but the only consumer just checks `!== undefined`).

While here — discovered + fixed independently while exhaustively probing
DX:

- **Replace `DISCARD ALL` with curated reset** (`RESET ALL; RESET
  SESSION AUTHORIZATION; UNLISTEN *; CLOSE ALL;`). DISCARD's
  `DEALLOCATE ALL` killed tokio_postgres' typeinfo cache, producing
  intermittent `prepared statement "sN" does not exist` errors on
  custom-type queries after cached-conn reuse. New regression tests:
  `test_postgresql_custom_types_on_cached_connection` and
  `test_postgresql_set_role_does_not_leak_across_cached_connection`
  (the latter catches the case where someone might switch back to
  `RESET ALL` alone and silently introduce a permission-leak vector —
  RESET ALL doesn't cover SET ROLE / SET SESSION AUTHORIZATION).

- **ISO-8601 timestamp results** (`pg_cell_to_json_value`). Pre-fix
  `TIMESTAMP` was rendered with a space separator ("2024-01-15 10:30:00")
  and `TIMESTAMPTZ` with " UTC" suffix ("2024-01-15 10:30:00 UTC") —
  neither parseable by `date-fns parseISO`, JavaScript `new Date()` is
  lenient enough to handle them but several frontend `App*Input.svelte`
  components use parseISO and fail silently. Switched to ISO-8601 with
  `T` separator and `+00:00` offset; arg-parsing path still accepts the
  legacy " UTC" suffix for back-compat.

Test coverage:
- 17/17 unit (`pg_executor::tests`)
- 9/9 integration (`backend/tests/worker.rs`, `test_postgresql_*`)
- 27/27 parser (`windmill-parser-sql`)
- 42/42 SDK (`typescript-client/tests/sqlUtils.test.ts`)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg): bounded one-shot warning on numeric precision loss + ISO-8601 + NaN handling

Found while probing PG-script DX with millions of numeric cells:

1. **Numeric precision-loss warning**: `numeric` results are still serialised
   as JSON Number (back-compat — switching to JSON String would silently
   break user code doing arithmetic on results), but we now detect
   `Decimal -> f64 -> Decimal` round-trip failure and emit a single
   job-log warning recommending a `::text` cast in the SQL. Bounded by
   `NUMERIC_PRECISION_CHECK_BUDGET = 256` cells per query (one atomic
   load + one fetch_sub on the hot path; first lossy value
   short-circuits to a single load thereafter). Worst-case overhead on
   a 1M-cell numeric-heavy query: ~25µs of checks + 5ns × N atomic
   loads (vs. ~100ms unbounded).

2. **ISO-8601 timestamps**: `pg_cell_to_json_value` previously returned
   `"2024-01-15 10:30:00"` (TIMESTAMP) and `"2024-01-15 10:30:00 UTC"`
   (TIMESTAMPTZ) — neither parseable by date-fns `parseISO`, which is
   what the apps `App*Input.svelte` components use, so timestamp values
   silently failed to round-trip into date pickers. Switch to ISO-8601
   (`T` separator + `+00:00` offset) on the result side; arg-parser
   continues to accept the legacy `" UTC"`-suffixed format for
   back-compat.

3. **Float NaN / Infinity results**: `Number::from_f64` returns None for
   NaN / ±Inf, which `pg_cell_to_json_value` was raising as
   "invalid json-float" — failing the *entire* query if any cell held
   one of these special values. Now serialise them as JSON strings
   ("NaN", "Infinity", "-Infinity") and let the rest of the row come
   through. Arg-side: `s.parse::<f64>()` already accepts the same
   strings.

Tests:
- `decimal_fits_f64_losslessly_predicate` — covers fits / doesn't-fit
  cases for the precision-loss predicate.
- `precision_check_budget_caps_per_query_overhead` — locks in the
  budget cap and the loss-flag short-circuit.
- All 9 PG integration tests + 17 unit tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg): add pg_advisory_unlock_all to reset; warn on missing args; honor decl defaults

While probing PG-script DX further found three more frictions:

1. **Advisory lock leak** (cubic P2): switching from `DISCARD ALL` to
   `RESET ALL; RESET SESSION AUTHORIZATION; UNLISTEN *; CLOSE ALL;`
   meant session-scoped advisory locks (`pg_advisory_lock`) leaked
   across cached-connection reuse. Add `SELECT pg_advisory_unlock_all()`
   to the chain — `DISCARD ALL` covered this implicitly via
   `DISCARD PLANS / DEALLOCATE / pg_advisory_unlock_all` and we lost it
   in the switch.

2. **Missing-arg silent NULL**: an arg declared in the SQL (e.g.
   `-- $1 amount (numeric)`) but not provided in the args object was
   bound as NULL with no error / warning. Misspelling the key in the
   args object silently produced a row of NULLs — a notorious DX
   debugging trap. Now: collect the names of declared-but-missing
   args during dispatch and emit a single one-shot warning to the job
   logs at end-of-query naming each one. Bound NULL is preserved for
   back-compat.

3. **Declaration defaults ignored**: `-- $1 a (int) = 5` carries
   `arg.default = Some(Number(5))`, but the dispatch fell straight to
   NULL when the arg was missing. Now: respect the default —
   user-supplied value > declaration default > NULL. Also fixes the
   warning logic above (only warn for args that *don't* have a default).

Tests: existing 19 unit + 9 integration pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pg): multi-word PG types with [] suffix lost the array-ness; array arms accept stringified values

Two more frictions found while probing SDK end-to-end against a real
datatable resource:

1. **Multi-word array types lose the [] suffix in the parser**.
   `transform_types_with_spaces` recognises aliases for "double
   precision", "character varying", "timestamp with time zone", etc.
   but its return type was `&'a str` — only the bare alias, never with
   a trailing `[]`. The `RE_CODE_PGSQL` regex's `\w+` captures stop at
   the first space, so the regex's own `(?:\[\])?` array-suffix branch
   sees only `"double"` (not `"double precision[]"`); the `[]` was
   silently lost. Result: `$1::double precision[]` (which the SDK now
   emits for homogeneous float arrays via the new auto-tag) routed
   through `Value::Array → Type::JSONB` and the server failed with
   "cannot cast type jsonb to double precision[]".

   Fix: switch `transform_types_with_spaces` to return `Cow<'a, str>`
   and re-check the trailing bytes after a multi-word match. If they
   start with `[]`, return `format!("{alias}[]")` — Owned. Single-word
   types and the no-match path keep returning Borrowed slices, so no
   allocation in the hot path.

2. **Array arms in `convert_vec_val` rejected stringified values for
   numeric / int* / bool / oid / real / double**. The scalar `convert_val`
   already parses strings into the matching native type for these arg_ts,
   but the array variant only accepted JSON-native counterparts. Sending
   `["1.5", "2.5", "3.5"]` against `$1::numeric[]` (e.g. via `unnest` for
   bulk loading, or `JSON.stringify(BigInt[])` round-trip) failed with
   "Mixed types in array". Now the array arms mirror the scalar ones —
   `as_<native>().or_else(|| as_str().and_then(parse))` — so both shapes
   round-trip cleanly.

Tests: 19 unit + 9 integration pass; existing parser tests cover the
multi-word array forms (the regex-cap behaviour didn't break for
single-word types, and Cow plumbing is transparent to all callers).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(parsers): add otyp_inferred field to Arg literals in tests + 3 missed src files

CI failures: the perl-driven sweep that added `otyp_inferred: false` to
every `Arg { ... }` literal when I introduced the field in the parser
schema covered `src/lib.rs` files but missed:

  - parsers/windmill-parser-bash/src/lib.rs       (mass-edited but a
    later format pass un-applied a few sites)
  - parsers/windmill-parser-go/src/lib.rs         (same)
  - parsers/windmill-parser-graphql/src/lib.rs    (same)
  - parsers/windmill-parser-nu/tests/tests.rs     (test file — not
    swept the first time)
  - parsers/windmill-parser-ts/tests/tests.rs     (test file — same)

Also tightened the regex to handle `oidx: None` without the trailing
comma (some test files had the field as the last initialiser line).

`cargo build --features <CI feature combo> --workspace --all-targets`
is clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sdk): Date → TIMESTAMPTZ; NaN / ±Infinity → string

Two more frictions found while running the actual SDK end-to-end against
a live datatable resource:

1. **JS `Date`** fell into the typeof "object" branch and was tagged
   `::JSON`. It worked accidentally for `${date}::timestamptz` via PG's
   `json → text → timestamptz` implicit cast chain, but `${date}` against
   a `timestamptz` column without a user-supplied cast bound the value
   as a JSON string and the comparison `timestamptz = json` failed. Now:
   `inferSqlType` recognises `Date` and tags `::TIMESTAMPTZ`;
   `serializeArgValue` emits `Date.toISOString()` so the executor's
   `Value::String → TIMESTAMPTZ` arm parses it cleanly.

2. **JS `NaN` / `±Infinity`** silently became NULL. `JSON.stringify(NaN)`
   returns `"null"` per the JS spec, so the value reached the executor as
   JSON null — the SDK's `::DOUBLE PRECISION` tag then bound a NULL
   double. Fix: detect non-finite numbers in `serializeArgValue` and
   stringify them as `"NaN" / "Infinity" / "-Infinity"`. The executor's
   `Value::String → FLOAT8` arm (`f64::from_str`) accepts these literals
   directly, and the result-side already renders the values as JSON
   strings (matching round-trip).

SDK unit tests grow from 42 → 44 passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(pg): integration coverage for multi-word arrays + stringified array elements

Locks in the two array fixes from the previous commit
(`fix(pg): multi-word PG types with [] suffix lost the array-ness`)
with end-to-end cases in `test_postgresql_arg_type_combinations`:

- `double precision[]`, `character varying[]`, `timestamp without time
  zone[]` — verifies the parser keeps the `[]` suffix after multi-word
  alias resolution.
- `numeric[]` / `int[]` / `bool[]` from stringified primitives — verifies
  the array arms of `convert_vec_val` apply the same string-coercion
  the scalar arms do.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* style: fix indentation drift on otyp_inferred lines

cargo fmt cleanup of leftover indentation where the perl-driven sweep
that introduced the otyp_inferred field landed at the wrong column.
No behaviour change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
 2026-05-01 17:08:59 +00:00
Yoaquim Cintrón 7bf6ac2b69 fix: enrich OTEL spans with job_kind, trigger_kind, trigger, created_by, and script_hash (#8718) 
Add five new attributes to the `job` and `job_postprocessing` tracing spans
so that OTEL-consuming backends (Sentry, Honeycomb, Datadog, etc.) can
filter and group telemetry by how a job was triggered and what type it is.

New span attributes:
- `job_kind`     — Script, Flow, AppScript, AIAgent, Preview, etc.
- `created_by`   — the user or system identity that queued the job
- `trigger_kind` — schedule, webhook, kafka, http, sqs, etc.
- `trigger`      — the schedule/trigger path (when applicable)
- `runnable_id`  — the id of the runnable that ran

Also adds `JobKind::as_str()` for a consistent lowercase string
representation, following the same pattern as `ScriptLang::as_str()`.

Existing attributes (job_id, workspace_id, script_path, language, tag,
flow_step_id, parent_job, root_job) are unchanged.

Note: the EE `full_job` span in `otel_ee.rs` and the log records emitted
by `job_logger_ee.rs` would also benefit from these attributes. This PR
covers only the public-repo spans; a follow-up EE change would propagate
the same fields to logs and the full_job span.
 2026-04-05 14:11:22 +00:00
Ruben Fiszel 5e5da4f7ef test: add OTEL coverage tests (#8558) 
* test: add OTEL coverage tests

Add 38 unit tests covering OpenTelemetry infrastructure:
- OtelSettings serde (empty, partial, full, roundtrip, skip_serializing)
- OtelTracingProxySettings serde (defaults, languages, dedup, rejection)
- ScriptLang rename cases
- LogCounter initialization and CountingLayer event counting
- Targets filter suppression of windmill:job_log
- get_otel_context_envs traceparent format verification
- Worker OtelTracingProxySettings (HashSet variant)

Companion EE PR adds tests for span_cx_from_job_id, metric functions,
proto conversion, SpanBuilder, and tracing proxy handler.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add E2E OTEL tests with in-memory exporters

Add integration tests that verify metrics and spans flow correctly
through the OpenTelemetry pipeline using in-memory exporters:

Metrics (1 comprehensive test):
- All 20 metric names registered correctly
- Counter values (push/delete/pull/zombie/execution/failed/started)
- Gauge values with attributes (queue count by tag, worker busy, db pool, health)
- Histogram values (execution duration, pull duration)
- Health status phase encoding (healthy=1, degraded=0, unhealthy=0)

Spans (6 tests):
- Root job span created with "full_job" name and Ok status
- Error status with "Job failed" description on failure
- trace_id derived from job UUID
- span_id derived from job UUID low bits
- Child jobs (with parent_job) produce no span
- Attribute values (job_id, workspace_id, script_path) match job data

Also:
- Add testing feature to opentelemetry_sdk for InMemoryMetricExporter
- Update otel_oss.rs for SdkTracer type rename in 0.30
- Add opentelemetry/opentelemetry_sdk to dev-dependencies

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: remove unit tests in favor of E2E OTEL tests

The E2E integration tests in backend/tests/otel.rs cover the same
ground more thoroughly with in-memory exporters.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-03-27 18:28:59 +00:00