feat: add feedback banner and issue links to errors - #12
Merged
Conversation
Bump posthog-telemetry to the analysis-first API. Register product identity and the deployment group at load via SetProduct + AssociateGroup, and migrate CaptureFunctionExecution -> RecordFunctionCall across all function binds. Add TELEMETRY.md documenting what is collected and the privacy guarantee.
OAuth breaks against real IdP configurations we cannot reproduce here -- Entra app roles, Google's string-typed numeric claims, Keycloak realms -- so the users who hit a problem are the only ones who can tell us what it was. Two surfaces now point at the tracker. A once-a-day banner on interactive load, from the shared DataZooDE/duckdb-extension-banner submodule. DuckDB has no notice channel, so it can only go to stderr, which makes the gating the load-bearing part: stderr must be a terminal, no CI marker set, DATAZOO_NO_BANNER unset, SET datazoo_banner not false, and the ~/.duckdb stamp over a day old. Piped output, notebooks, CI and the test suite therefore see nothing, so no existing expected output changes. An issue link on every error raised by a registered function, via DATAZOO_GUARD at the eleven registration sites. The guard yields a plain function pointer with the same signature, preserves the original ExceptionType, and leaves INTERRUPT, FATAL and OUT_OF_MEMORY alone. Errors DuckDB raises before dispatch -- binder, catalog, arity -- are deliberately left unannotated; they are not ours, and claiming them would send users to the wrong tracker. quack_oauth_feedback.test pins that. The banner include is placed unconditionally rather than after the last #include in each file: in check_authorization_function.cpp and diagnose.cpp the latter would have landed inside #ifndef EMSCRIPTEN and broken the wasm configuration. Suite green: 168 assertions in 11 cases.
This was referenced Aug 6, 2026
The banner library declared INTERFACE cxx_std_17. That propagates through the extension target into DuckDB's own tools/plan_serializer, which then compiles as C++17 while libduckdb_static stays C++11 -- BufferedFileWriter::DEFAULT_OPEN_FLAGS ends up COMDAT-weak on one side and strong on the other, and the link fails with a multiple definition error. anofox-statistics CI caught it; the declaration has been removed upstream since every consumer already builds at C++17. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
On MinGW the 'struct _stat64' tag hides the function of the same name in C++, so the banner's _stat64 call parsed as a constructor and every rtools/MinGW Windows job failed to compile. The shared library now uses plain stat() everywhere except MSVC. Caught by anofox-similarity CI, which builds windows_amd64_rtools.
# Conflicts: # posthog-telemetry
The merge resolution left this branch's older pin (e41682b, 12 Jul) in place, which would have regressed main's newer f550a66 (16 Jul) on merge. git checkout --theirs silently skips submodules, so the conflict looked resolved while the wrong commit stayed staged. f550a66 contains e41682b, so taking main's is strictly newer and keeps the schema-2 telemetry work on this branch intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OAuth breaks against real IdP configurations we cannot reproduce here — Entra app roles, Google's string-typed numeric claims, Keycloak realms — so the users who hit a problem are the only ones who can tell us what it was. Two surfaces now point at the tracker.
Part of a fleet-wide rollout; implementation is the shared
DataZooDE/duckdb-extension-bannersubmodule, so the wording is fixed in one place. Same pattern as erpl-tunnel#2 and erpl-adt#36.What changed
Once-a-day banner on interactive load. Prints only when stderr is a terminal, no CI marker is set,
DATAZOO_NO_BANNERis unset,SET datazoo_banneris not false, and the~/.duckdbstamp is over a day old. Piped output, notebooks, CI and the test suite see nothing — which is why no existing expected output changes.Issue link on every error raised by a registered function, via
DATAZOO_GUARDat the 11 registration sites. Preserves the originalExceptionType; leavesINTERRUPT,FATAL,OUT_OF_MEMORYalone.Two things worth reviewing
Errors DuckDB raises before dispatch are deliberately not annotated. Binder, catalog and arity errors are not ours, and claiming them would send users to the wrong tracker.
quack_oauth_feedback.testpins this.The banner include is placed unconditionally, not after each file's last
#include. Incheck_authorization_function.cppanddiagnose.cppthe latter would have landed the include inside#ifndef EMSCRIPTENand broken the wasm configuration — the identical mistake broke erpl-tunnel's SSH-only and musl builds before I caught it.Verified
quack_oauth_feedback.test(5 assertions)duckdbsubmodule pin untouched (the dirty marker is the pre-existing configure-time fmt patch)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.