Open
Conversation
First red-green cycle for #201. Introduces `common.core.docgen.events.get_event_entries_from_source` plus `EventEntry` / `SourceLocation` data classes, handling the simplest shape: a literal-domain `structlog.get_logger("X")` assignment followed by `var.<level>("evt")` produces one entry. Further cycles layer `__name__` domain resolution, the level allowlist, bind mechanics, cross-site merging, filesystem walk, and the management command + template. beep boop
Second cycle for #201. Kwargs on the emit call become the entry's `attributes` set, with `__` → `.` substitution so the catalogue reads in the dotted form product stakeholders expect. beep boop
Third cycle for #201. A `structlog.get_logger(__name__)` call now resolves the domain statically to the module dotted path passed into the scanner, so loggers without an explicit string domain still produce catalogue entries. beep boop
Fourth cycle for #201. Adds `DocgenEventsWarning` and emits it with file + line context when a `structlog.get_logger(X)` argument is neither a string literal nor `__name__`. The logger is skipped rather than crashing the build, per the issue's acceptance contract. beep boop
Fifth cycle for #201, characterization-only. The `structlog.`-prefix gate on logger assignments already excludes stdlib loggers; this test pins that contract so a future refactor can't silently widen the scan to include them. beep boop
Sixth cycle for #201. An emit call whose first positional argument is not a string literal now emits `DocgenEventsWarning` with the call site rather than being silently dropped. The warning suggests the `# docgen: event=<name>` annotation as a future escape hatch for intentional dynamic names. beep boop
Seventh cycle for #201. Attribute calls on a tracked logger now only produce entries when the method name is one of the emission methods that `structlog.stdlib.BoundLogger` actually exposes (`debug`, `info`, `warning`, `warn`, `error`, `critical`, `fatal`, `exception`, `msg`). Other methods are ignored silently so helper calls don't contaminate the catalogue. `exception` is retained because `launch_darkly/tasks.py` — one of the five acceptance-listed domains — emits `log.exception("import-failed")` as its canonical error pattern. `log(level, msg, ...)` is excluded: its first arg is the level, not the event name. beep boop
Phase A closure cycle for #201. Every scanner behaviour — positive, silently-ignored, and warn-and-skip — now lives in a single `test_get_event_entries_from_source__emit_log__expected_entries` parametrise. Each case specifies the source, expected entries, and expected `DocgenEventsWarning` instances (exact message match), so message regressions fail the suite rather than slipping through a substring match. Branch coverage on `events.py` stays at 100%. beep boop
First cycle of phase B for #201. Introduces the internal `_LoggerScope(domain, bound_attrs)` and replaces the flat `logger_domains` map with `logger_scopes`. An assignment of the shape `log = <tracked>.bind(k=v, ...)` now registers `log` as a new scope carrying the parent's domain plus the extra bound attributes. Emission calls on `log` union those bound attrs with the call's own kwargs, so a `log.info("success", response_status=200)` rendered after `log = logger.bind(sentry_action=..., feature_name=...)` captures all three attributes in the catalogue. Further phase-B cycles will cover chained binds, multi-chained binds, self-reassignment, and cross-function scope isolation. beep boop
Cycle B2 for #201. Emission calls may now target a `<logger>.bind(...)` chain rather than only a bare Name, so `logger.bind(a=1).info("e", b=2)` picks up both `a` and `b` as attributes. Works recursively, setting up multi-chain support in the next cycle. The dynamic-event-name warning now describes the chain shape (e.g. ``logger.bind(...).info(...)``) so the call-site is locatable from the message alone. beep boop
Cycle B3 for #201, characterization-only. The recursive `_scope_for_emit_target` added in the previous cycle already handles arbitrary bind depth; this test pins that contract so `logger.bind(a=1).bind(b=2).info("e", c=3)` can't silently regress to only capturing the innermost bind. beep boop
Cycle B4 for #201, characterization-only. `log = logger.bind(a=1); log = log.bind(b=2)` already works because `_resolve_bind` looks up the right-hand side in `logger_scopes` and the second assignment finds the previously registered `log`. This test pins that chain so a refactor can't silently drop the second bind. beep boop
Cycle B5 for #201. Replaces the single-pass flat-dict collection with an `ast.NodeVisitor` that pushes a copy of the enclosing scope on entry to a `FunctionDef`/`AsyncFunctionDef` and pops on exit. Binds inside one function no longer overwrite the same name in a sibling function — so a pair of `def`s that each do `log = logger.bind(...)` with different kwargs now produce two entries with their own attribute sets, rather than the second bind leaking into the first. Assignments and calls are processed in source order by the visitor, which also removes the two-pass design in favour of a single walk. beep boop
Phase B closure for #201. `structlog.get_logger()` (no args) is not a skippable case — the OTel structlog processor in `common.core.otel` emits those events without a namespace prefix (see the `event_dict.get("logger")` branch there), so the scanner registers them with an empty domain and emits entries whose name is the bare event string. Also adds the six other parametrised cases needed to close Phase B branch coverage: a bind inside `async def`, a dynamic event name rendered through the chain describer (warning text reads ``logger.bind(...).info(...)``), bind on an untracked name, chain with a non-bind middle attribute, chain whose base isn't a tracked logger, and a subscript target that should just be ignored. The unreachable `<logger>` fallback in `_describe_emit_target` goes too — scope resolution already narrows the input to a Name or a bind-chain Call, so the helper asserts that invariant rather than papering over it. Branch coverage on `events.py` back to 100% before phase C (cross-site merging). beep boop
Handles the common flagsmith pattern where a class method is used as
a bound-logger accessor:
class Worker:
def logger(self, project_id):
return logger.bind(project__id=project_id)
def do_work(self, project_id):
self.logger(project_id=project_id).info("project.worked")
`visit_ClassDef` pre-scans the body for methods whose only
non-docstring statement is `return <tracked>.bind(...)` (or a
resolvable bind chain) and registers the method name in a per-class
scope. `_scope_for_emit_target` learns to recognise `self.<name>` and
`self.<name>(...)` (also `cls.` for classmethods) and resolves them
against that class scope. The emit's kwargs still union with the
method's bound attrs.
Limitations called out in later cycles: no cross-class inheritance
yet (C-edge-2 adds same-file inheritance), no cross-file (won't ship
in v1), no warning for unresolvable self/cls emits yet (C-edge-3).
beep boop
…rning
Extends class-scope method-accessor support:
* `visit_ClassDef` registers each class in a module-level
`{class_name: class_scope}` map and, when a child class's bases
include a same-file Name-typed parent, inherits that parent's
accessors (child's own methods override).
* `cls` is already treated like `self` for accessor lookup
(classmethod-friendly).
* Property-style `self.<name>.info(...)` access now resolves through
the same class scope as the call-form `self.<name>(...).info(...)`.
* Methods with leading docstrings register as accessors.
* Unresolved `self.<X>.<level>("literal", ...)` or
`self.<X>(...).<level>("literal", ...)` emissions now emit a
`DocgenEventsWarning` pointing the engineer at the inline-bind or
move-accessor-to-same-file escape hatches. Cross-file inheritance
is deliberately out of scope for v1 — the warning ensures those
misses aren't silent.
beep boop
Adds `merge_event_entries(entries)` which collapses entries sharing an event name: unions the attribute sets, concatenates the locations, keeps the first-seen log level, and returns the result sorted alphabetically by event name. Diverging log levels for the same event name emit a `DocgenEventsWarning` naming both call sites and the level that won, so the engineer can reconcile the emission sites. Sets up phase D (filesystem walk) — which will feed per-file entries through this merger to produce the single, deduplicated catalogue the docgen command renders. beep boop
Adds `get_event_entries_from_tree(root, *, app_label, module_prefix)` which rglobs every `*.py` under `root`, computes the module dotted path for each file (via `module_prefix`), and pipes it through `get_event_entries_from_source`. Skips: - `migrations/` and `tests/` anywhere in the path. - `conftest.py` and `test_*.py` at any depth. - `management/commands/` unless `app_label == "task_processor"` — the task processor app emits operationally important runner-loop events from that directory. Combined with `merge_event_entries` and the per-source scanner, this is everything the docgen management command needs from the scanner layer. Phase F next: template + `flagsmith docgen events` subcommand + integration snapshot test. beep boop
Final layer of #201. Adds a Jinja template at `templates/docgen-events.md` mirroring the existing metrics template, and a new `events` subparser on the `docgen` management command that walks every app returned by `apps.get_app_configs()`, merges entries with `merge_event_entries`, and renders the template. Source paths are rendered relative to the git repo root (queried via `git rev-parse --show-toplevel`), with a CWD fallback when the command is invoked outside a git repo. Paths that aren't under the resolved root stay absolute. Three integration tests cover each path through the public CLI, no private helpers under test: - `fixture_app` / `patched_apps` pytest fixtures stand up a two-file tree under `tmp_path/api/fixture_app` and wire the docgen command to scan only it. - `test_docgen__events__runs_expected` git-init's the fixture tree and snapshot-compares stdout against the rendered catalogue. - `test_docgen__events__falls_back_to_cwd_when_not_in_git_repo` omits the `git init`; asserts cwd-relative paths are emitted. - `test_docgen__events__keeps_absolute_path_when_app_outside_repo_root` init's a sibling subtree as the git repo; asserts absolute paths pass through unchanged. Both `events.py` and `docgen.py` at 100% branch coverage. beep boop
khvn26
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #202 +/- ##
==========================================
+ Coverage 97.05% 97.26% +0.20%
==========================================
Files 102 104 +2
Lines 4140 4457 +317
==========================================
+ Hits 4018 4335 +317
Misses 122 122 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When Django's INSTALLED_APPS contains both a parent app and a nested sub-app, the same source file gets scanned via both app roots and emits two identical EventEntry objects. The merge step then stacked their location lists, producing duplicate `path:line` entries in the rendered catalogue. Collapse identical locations on merge so overlapping app layouts stay idempotent. beep boop
khvn26
added a commit
to Flagsmith/flagsmith
that referenced
this pull request
Apr 17, 2026
Generates `docs/.../events.md` via `flagsmith docgen events` — the new subcommand shipped in Flagsmith/flagsmith-common#202. The catalogue lists every structured event the backend emits, the log level each is emitted at, its source locations, and the attributes it carries, so product and ops can curate the OTel allowlist without grepping the codebase. Wired into `make generate-docs` so the catalogue stays in sync with the source on the existing pre-commit hook — any PR that adds or changes an emit surfaces the diff in review. Points `flagsmith-common` at the PR branch for smoke testing; switch to a released version before merging. beep boop
4 tasks
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
2 participants
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.
Contributes to #201.
Product needs a canonical, checked-in list of the structured events the backend emits, so the OTel allowlist can be curated with full visibility and so new or changed events surface in PR review rather than being discovered later via grep. There's no structlog equivalent of Prometheus's runtime registry, so the catalogue has to be generated from static analysis.
This adds
flagsmith docgen events, a subcommand alongsidedocgen metrics, that walks every installed app, resolves each structlog emit back to its event name, log level, attributes, and source location, and renders the result into a sorted markdown catalogue. Level is load-bearing because structlog only reaches OTel when the event passes the configured level, so a debug-level emit is invisible in production and that needs to be obvious in the catalogue.The scanner understands the patterns that come up in the backend: literal and
__name__logger domains, empty-domain loggers, bind chains across function scopes, class methods that return bound loggers, same-file inheritance and classmethod usage. When it can't resolve a call site, it warns loudly rather than silently dropping the event. Cross-file inheritance is left out of v1 on purpose, with a warning pointing the engineer at inlining or moving the accessor.Follow-ups, not in this PR: rendering source paths as links to GitHub, the
make generate-docstarget and pre-commit enforcement in flagsmith/flagsmith, and the CODEOWNERS wiring for the catalogue page.Test plan