Skip to content

feat: organization-scoped event custom fields with management UI and filter integration - #1815

Draft
ejsmith wants to merge 32 commits into
mainfrom
feature/custom-fields
Draft

feat: organization-scoped event custom fields with management UI and filter integration#1815
ejsmith wants to merge 32 commits into
mainfrom
feature/custom-fields

Conversation

@ejsmith

@ejsmith ejsmith commented Feb 2, 2025

Copy link
Copy Markdown
Member

Summary

Adds explicit, opt-in, organization-scoped typed event custom fields backed by pooled Elasticsearch slots, with API, MCP, saved-view, and Svelte management/filter integration.

This remains a forward-only indexing cutover. User fields are indexed only after a definition exists; historical events are not backfilled. The only dual-read compatibility fields are Exceptionless-owned @ref:session, sessionend, and haserror.

Production hardening

  • Uses one canonical custom-field query policy across event/event-stack APIs, saved views, and MCP. Unknown definitions and raw slots fail clearly; custom queries require exactly one organization.
  • Resolves repository tenants from single-organization AppFilter scopes and preserves the explicit free-organization global-admin path through ApiFilterPolicy.
  • Predeclares the bounded pooled mappings, preserves legacy suffix templates, installs and validates them across retained daily event indexes behind a schema/day distributed startup gate, and fails readiness on incompatible mappings.
  • Builds replacement slots off-document, strips untrusted client slots on new events, aborts saved-event writes on infrastructure failures, and requires finite/in-range/integral conversions where appropriate.
  • Enforces active and lifetime allocation ceilings with typed API outcomes and low-cardinality diagnostics.
  • Uses PATCH presence semantics and validation, sorted GET results, synchronous DELETE 204, parsed saved-view references, and shared organization locks for filter/definition mutation integrity.
  • Rejects organization-specific fields in both canonical and structured global predefined-view filters.
  • Keeps custom-field UI state organization-owned, cleans up dynamic filter registrations, preserves exact long literals as strings, and handles downgraded organizations read-only.

Customer regression

Focused HTTP ingestion coverage configures DatabaseVersion as keyword, posts both "4.90" and "4.90 build 1234 30-Aug-2024", reloads Elasticsearch _source, verifies exact Data/pooled Idx strings, and proves 4.90 matches while 4.9 does not. A double definition intentionally converts "4.90" to 4.9 and skips the development string without altering raw Data.

Upgrade and rollout

  • There is no built-in historical custom-field backfill. Elasticsearch reindexing alone does not populate pooled slots.
  • Replaying original payloads can create duplicate events unless operators provide their own deduplication strategy.
  • Drain older writers before the UTC partition rollover, deploy ingestion/API first, wait for the shared mapping readiness gate, seed definitions, then expose the UI only after exact/range/facet canaries pass.
  • Roll forward on failure. Preserve definitions, slots, and raw event data; never hard-delete definitions or reclaim slots during incident response.

Verification at e53fbcce1

  • Backend build: succeeded, 0 warnings/errors.
  • Full backend/Elasticsearch CI: 2,845/2,845 tests passed, including OpenAPI snapshots and endpoint manifests.
  • EventCustomFieldServiceTests: 71/71 passed.
  • PersistentEventCustomFieldsTests: 14/14 passed.
  • Frontend Vitest: 40 files / 422 tests passed locally and in CI.
  • Frontend lint, Svelte check, validate, and production build passed.
  • Aspire/Playwright E2E passed on the final head.
  • Docker image builds, website build, docs static check, and git diff --check passed.

Local Docker/Aspire dogfood was unavailable because Docker Desktop was unhealthy. GitHub CI provides the Elasticsearch-backed ingestion/query and Aspire browser proof; representative retained-index startup timing remains a staging gate.

Production blockers / known limitations

  • Do not merge for production yet: the branch still consumes Foundatio.Repositories.Elasticsearch 8.0.2-preview.elasticsearch-mapping-issue-investigation.0.3. An official immutable release must contain the required mapping behavior and later large-mapping fix, then this PR must pin that release.
  • Validate mapping-startup timing and incompatible-mapping fail-fast behavior against representative retained-index scale.
  • No historical backfill or legacy user-field read bridge; soft-deleted slots are not reclaimed.

Breaking behavior

Arbitrary extended-data fields without an active organization definition are no longer automatically indexed or searchable through data.*. Existing saved views and integrations relying on automatic user-field indexing need a one-time definition rollout. Existing API shapes remain compatible.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds custom field support for events by updating the event model and repository logic, removing legacy query visitor code, and updating test infrastructure to support the new behavior.

  • Removed obsolete EventFieldsQueryVisitor usage and file.
  • Introduced automatic custom field creation in the event repository.
  • Updated the PersistentEvent model to implement virtual custom fields and modified test configurations accordingly.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/Exceptionless.Tests/Search/PersistentEventQueryValidatorTests.cs Removed legacy query visitor call in tests.
tests/Exceptionless.Tests/Search/EventIndexTests.cs Updated repository query to include organization filtering.
tests/Exceptionless.Tests/Migrations/FixDuplicateStacksMigrationTests.cs Set log level to Trace for migration tests.
tests/Exceptionless.Tests/AppWebHostFactory.cs Added Kibana container configuration.
src/Exceptionless.Core/Repositories/Queries/Visitors/EventFieldsQueryVisitor.cs Removed unused visitor implementation.
src/Exceptionless.Core/Repositories/EventRepository.cs Added auto-creation of custom fields and related tenant logic.
src/Exceptionless.Core/Repositories/Configuration/Indexes/EventIndex.cs Removed legacy dynamic mapping for event index custom fields.
src/Exceptionless.Core/Repositories/Configuration/ExceptionlessElasticConfiguration.cs Added index creation for custom fields.
src/Exceptionless.Core/Models/PersistentEvent.cs Updated Idx property type and implemented IHaveVirtualCustomFields.
Files not reviewed (1)
  • src/Exceptionless.Core/Exceptionless.Core.csproj: Language not supported

Comment thread src/Exceptionless.Core/Models/PersistentEvent.cs Outdated
@niemyjski niemyjski changed the title Add custom fields for events feat: organization-scoped event custom fields with management UI and filter integration May 24, 2026
Comment thread src/Exceptionless.Core/Models/PersistentEvent.cs Fixed
Comment thread src/Exceptionless.Core/Services/EventCustomFieldService.cs Fixed
Comment thread src/Exceptionless.Core/Services/EventCustomFieldService.cs Fixed
Comment thread src/Exceptionless.Core/Services/EventCustomFieldService.cs Fixed
Comment thread src/Exceptionless.Core/Services/EventCustomFieldService.cs Fixed
Comment thread src/Exceptionless.Core/Services/EventCustomFieldService.cs Fixed
Comment thread src/Exceptionless.Core/Services/EventCustomFieldService.cs Fixed
@niemyjski
niemyjski requested a review from Copilot May 25, 2026 13:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Comment thread tests/Exceptionless.Tests/CustomFields/CustomFieldApiTests.cs Fixed
@niemyjski
niemyjski requested a review from Copilot May 28, 2026 11:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

…filter integration

Add organization-level custom field management that allows premium users to define
typed, searchable event fields (keyword, string, numeric, date, boolean) with full
lifecycle management including soft-delete, index slot reuse, and saved view protection.

Key changes:
- Custom field CRUD on organization controller with validation and plan gating
- EventCustomFieldService for document change handling and field lifecycle
- Premium feature detection in saved views (UsesPremiumFeatures flag)
- Filter integration with operator support per field type
- Angular UI: management dialog, filter picker integration
- Svelte UI: custom fields management and filter components
- Comprehensive test coverage (107+ custom field tests)
- Max 20 active fields per organization, slot-based index allocation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@niemyjski
niemyjski force-pushed the feature/custom-fields branch from 0a8855a to b277880 Compare May 28, 2026 17:44
niemyjski and others added 6 commits May 28, 2026 13:02
…seline, fix Prettier

- Restore accidentally deleted ControllerManifestTests.cs (was removed during squash)
- Generate controller-manifest.json baseline with custom fields endpoints included
- Fix Prettier formatting in sidebar.svelte

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…heck

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…finedAsync

The previous baseline was missing PostPredefinedAsync because UPDATE_SNAPSHOTS
ran against a stale build. Rebuilt and regenerated correctly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…es, ContainsKey pattern

- PersistentEvent.GetCustomFields(): consolidate value-type guard into the .Where() predicate
- EventCustomFieldService: use .OfType<PersistentEvent>() to replace the outer .Where() null filter
  and the redundant inner .Where(d => d is not null) in the document loop
- CustomFieldIndexingTests: fix regex type names (integer→int, boolean→bool; add string and float)
- CustomFieldApiTests: fix 'integer' index type to 'int' (integer is not a supported type)
- CustomFieldApiTests: replace ContainsKey+indexer with TryGetValue for SessionHasError assertion

The 'integer' index type bug was silent: ConvertValue returns null for unknown types so the
field would be stored but never actually indexed, with no error surfaced to the user.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ring

ConvertToKeyword and ConvertToString were calling value.ToString() without
a format provider. On servers with a non-en-US locale (e.g. de-DE), float,
double, and decimal values produce '1,5' instead of '1.5', breaking
Elasticsearch keyword-field searches because the stored value and the query
value use different decimal separators.  DateTime values were also formatted
with the current thread culture instead of ISO 8601.

Fix: introduce a FormatInvariant helper that uses CultureInfo.InvariantCulture
for numeric types and the roundtrip 'O' ISO-8601 format for DateTime/DateTimeOffset.
The bool.ToString() behaviour ("True"/"False") is preserved for backwards
compatibility with already-indexed data.

Add regression tests that explicitly set CultureInfo to de-DE and assert
that the output always uses an invariant decimal separator.  Also add tests
for long coercion in ConvertToBool, and for DateTime/DateTimeOffset keyword
formatting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@niemyjski

Copy link
Copy Markdown
Member

Focused custom-field and session direction

After re-reading the current implementation, I think the session compatibility concern is real, but the larger proposed redesign is more machinery than this PR needs.

Contract to preserve

  • User-defined fields remain explicitly opt-in, forward-only, and never backfilled or reindexed.
  • @ref:session, sessionend, and haserror are Exceptionless-owned system fields: automatically provisioned, hidden from custom-field management, excluded from the 20-field user quota, and protected from user edit/delete/reclamation.
  • The three system indexes should be maintained on all plans, while the existing premium gating for Sessions endpoints and searches remains unchanged.
  • System definitions should be removed during organization teardown along with the rest of the organization data; protection from user churn should not make them globally undeletable.

Changes required before merge

  1. Preserve retained session data on reads. The current resolver maps the logical system fields only to the new pooled slots. During the retention window we need dual reads:

    • ref.session -> idx.session-r OR idx.keyword-1
    • data.sessionend -> idx.sessionend-d OR idx.date-1
    • data.haserror -> idx.haserror-b OR idx.bool-1

    A complete session lookup must continue to include the top-level reference branch for session-start events. Exists should use legacy OR current, missing should use legacy AND current, and negation should wrap the combined expression. This is read compatibility only; no historical mutation or backfill should be introduced.

  2. Verify the deterministic slot invariant. Writes use the slot assigned to each definition, while queries currently assume keyword-1, date-1, and bool-1. Provisioning should verify the exact name, type, slot, tenant/entity scope, active state, and absence of a competing occupant. Invalid state should produce a clear deterministic conflict instead of silently creating or accepting a definition that queries cannot read.

  3. Centralize only the shared facts. A small descriptor containing source name, index type, reserved slot, and legacy index field would remove the current duplication between provisioning and query resolution. I do not think we need a flag-heavy registry or trusted metadata in CustomFieldDefinition.Data for this PR.

  4. Correct the compatibility claims. The architecture document already lists all three system fields, but the PR description lists only sessionend and haserror. It should include @ref:session and explain the separation between all-plan system indexing and premium Sessions access.

    The PR also cannot claim that this is fully additive and that existing queries are unaffected. Removing CopySimpleDataToIdxAction stops automatic indexing for unregistered Data keys, and removing EventFieldsQueryVisitor removes the old data.foo -> idx.foo-* resolution for retained events. Strict opt-in may still be the right product choice, but that cutover is a behavioral breaking change unless we define a migration contract. It needs to be acknowledged and explicitly accepted rather than hidden under the session fix.

Scope I do not think is required here

I would not require an administrative preflight feature, a global lifecycle lock, a persisted compatibility cutoff, an eight-stage deployment, new operator policy, or a general backfill/reindex system. Foundatio already serializes slot allocation per entity/tenant/type, and this branch already serializes user creation for quota enforcement. Exact invariant validation plus dual-read compatibility is the proportional fix.

Verification needed

  • A session lookup returns the session start plus both legacy and new child events.
  • Legacy and new session-end fields both classify open/closed sessions correctly.
  • haserror positive, exists, missing, and negated expressions preserve Boolean semantics across both layouts.
  • Wrong-slot, wrong-type, soft-deleted, duplicate, and occupied-slot system definitions fail deterministically.
  • The same session ID in two organizations remains tenant-isolated.
  • Raw pooled-slot queries remain blocked.
  • Free organizations write the three system indexes but remain blocked from premium Sessions functionality.
  • Twenty user fields remain available in addition to the three system fields.
  • Tests confirm that no historical event is updated, reindexed, or backfilled.

This keeps the useful part of the feedback—session continuity and deterministic reserved slots—without turning this PR into a broader custom-field control plane.

@niemyjski

Copy link
Copy Markdown
Member

Follow-up audit for the focused custom-field and session direction is complete.

Feedback disposition

  • All 10 inline review threads are resolved and outdated against the current diff.
  • Already fixed and revalidated: the DataDictionary fallback, declarative/null-filtered iteration, IsValidFieldName predicate, conversion exception narrowing, and TryGetValue/valid int test setup.
  • The three remaining broad operational catches are intentional and non-actionable: organization maintenance and event ingestion must continue after per-organization/per-event repository, transport, lock, or network failures, while OperationCanceledException still propagates. Narrowing them to guessed third-party exception families would reduce resilience.
  • Copilot “too many files”/review-error submissions and the coverage bot comment contain no actionable finding.

RCA and final fix

The session continuity concern was real: pooled system slots replaced retained legacy session indexes. The branch now centralizes the three system descriptors, validates their exact reserved invariants, and performs legacy/current dual reads with correct positive, exists, missing, range, and negation semantics.

The remaining valid issue was the public contract, not another migration framework: automatic arbitrary extended-data indexing was removed, but the PR description and filtering docs still called the change additive and even suggested a data.* historical-query workaround that resolves to the new slot. Commit 513270fd8:

  • documents the intentional forward-only cutover and one-time hosted/self-hosted rollout;
  • corrects the public filtering and self-host upgrade guidance;
  • adds an integration regression proving definition creation leaves a retained legacy event unchanged/unmatched while a post-definition event matches.

The PR description now reflects the current four endpoints, three system fields, synchronous soft-delete policy, known limitations, and breaking search behavior.

Verification

  • dotnet build tests/Exceptionless.Tests/Exceptionless.Tests.csproj --no-restore --nologo --disable-build-servers -m:1 — passed, 0 warnings/errors
  • isolated CustomFieldIndexingTests — 24/24 passed
  • docs deno task check — passed
  • docs deno task verify — build passed; 4/4 tests, telemetry validation, and local-link validation passed
  • git diff --check — passed

Thermo-nuclear structural review found no PR-caused file crossing 1,000 lines, no justified code-judo simplification beyond the existing small canonical system descriptor/visitor, and no reason to add a general backfill, dual-write, or migration control plane for this accepted one-time cutover.

# Conflicts:
#	src/Exceptionless.Web/ClientApp/src/lib/features/events/premium-filter.test.ts
#	src/Exceptionless.Web/ClientApp/src/routes/(app)/+layout.svelte
#	tests/http/organizations.http
# Conflicts:
#	src/Exceptionless.Web/ClientApp/src/routes/(app)/+layout.svelte
# Conflicts:
#	src/Exceptionless.Web/ClientApp/src/lib/features/shared/components/faceted-filter/faceted-filter-number.svelte.test.ts
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Complexity Health
Exceptionless.Core 75% 67% 10337
Exceptionless.AppHost 38% 40% 139
Exceptionless.Insulation 37% 35% 286
Exceptionless.Web 85% 69% 7274
Summary 78% (24238 / 31177) 67% (11448 / 17082) 18036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants