-
-
Notifications
You must be signed in to change notification settings - Fork 507
Include ref.parent matches in events by-reference navigation
#2280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f09562e
8f5e641
ae3cc52
311c5d5
6f152ed
775c14f
0e94773
5f0dd46
6942014
a507d66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| using System.Diagnostics; | ||
| using Elastic.Clients.Elasticsearch; | ||
| using Exceptionless.Core.Models; | ||
| using Exceptionless.Core.Repositories.Configuration; | ||
| using Foundatio.Repositories.Elasticsearch.Extensions; | ||
| using Foundatio.Repositories.Migrations; | ||
| using Microsoft.Extensions.Logging; | ||
|
|
||
| namespace Exceptionless.Core.Migrations; | ||
|
|
||
| public sealed class BackfillParentReferences : MigrationBase | ||
| { | ||
| private readonly ElasticsearchClient _client; | ||
| private readonly ExceptionlessElasticConfiguration _config; | ||
| private readonly TimeProvider _timeProvider; | ||
|
|
||
| public BackfillParentReferences(ExceptionlessElasticConfiguration configuration, TimeProvider timeProvider, ILoggerFactory loggerFactory) : base(loggerFactory) | ||
| { | ||
| _config = configuration; | ||
| _client = configuration.Client; | ||
| _timeProvider = timeProvider; | ||
|
|
||
| MigrationType = MigrationType.VersionedAndResumable; | ||
| Version = 3; | ||
| } | ||
|
|
||
| public override async Task RunAsync(MigrationContext context) | ||
| { | ||
| string referenceKey = $"@ref:{Event.KnownReferenceNames.Parent}"; | ||
| string indexKey = $"{Event.KnownReferenceNames.Parent}-r"; | ||
| string script = $"if (ctx._source.data != null && ctx._source.data.containsKey('{referenceKey}') && ctx._source.data['{referenceKey}'] != null) {{ if (ctx._source.idx == null) ctx._source.idx = [:]; ctx._source.idx['{indexKey}'] = ctx._source.data['{referenceKey}']; }} else {{ ctx.op = 'noop'; }}"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a retained event contains a non-string AGENTS.md reference: AGENTS.md:L72-L75 Useful? React with 👍 / 👎. |
||
|
|
||
| _logger.LogInformation("Backfilling retained event parent references"); | ||
| var stopwatch = Stopwatch.StartNew(); | ||
| var response = await _client.UpdateByQueryAsync<PersistentEvent>(request => request | ||
| .Indices($"{_config.Events.VersionedName}-*") | ||
| .Query(query => query.Bool(filter => filter.MustNot(mustNot => mustNot.Exists(exists => exists.Field($"idx.{indexKey}"))))) | ||
| .Script(value => value.Source(script).Lang(ScriptLanguage.Painless)) | ||
| .Conflicts(Conflicts.Proceed) | ||
| .WaitForCompletion(false)); | ||
| _logger.LogRequest(response, LogLevel.Information); | ||
|
|
||
| if (!response.IsValidResponse || response.Task is null) | ||
| throw new ApplicationException($"Unable to start parent-reference backfill: {response.DebugInformation}"); | ||
|
|
||
| int attempts = 0; | ||
| while (!context.CancellationToken.IsCancellationRequested) | ||
| { | ||
| var taskStatus = await _client.Tasks.GetAsync(response.Task.FullyQualifiedId, context.CancellationToken); | ||
| if (!taskStatus.IsValidResponse) | ||
| throw new ApplicationException($"Unable to monitor parent-reference backfill: {taskStatus.DebugInformation}"); | ||
|
|
||
| if (taskStatus.Completed) | ||
| { | ||
| _logger.LogInformation("Finished parent-reference backfill: Duration={Duration}", stopwatch.Elapsed); | ||
| return; | ||
| } | ||
|
|
||
| attempts++; | ||
| await context.Lock.RenewAsync(); | ||
| await Task.Delay(TimeSpan.FromSeconds(attempts <= 5 ? 1 : 5), _timeProvider, context.CancellationToken); | ||
| } | ||
|
|
||
| context.CancellationToken.ThrowIfCancellationRequested(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,7 +245,8 @@ export class ReferenceFilter implements IFilter { | |
| return ''; | ||
| } | ||
|
|
||
| return `reference:${quoteIfSpecialCharacters(this.value)}`; | ||
| const reference = quoteIfSpecialCharacters(this.value); | ||
| return `(reference:${reference} OR ref.parent:${reference})`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this filter is used on the events route, Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { filterUsesPremiumFeatures } from './premium-filter'; | ||
|
|
||
| describe('filterUsesPremiumFeatures', () => { | ||
| it('keeps built-in parent reference navigation available without premium features', () => { | ||
| expect(filterUsesPremiumFeatures('(reference:"parent-id" OR ref.parent:"parent-id")')).toBe(false); | ||
| }); | ||
|
|
||
| it('still requires premium features for custom references', () => { | ||
| expect(filterUsesPremiumFeatures('ref.custom:"reference-id"')).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an existing caller uses a custom reference name that was previously accepted, such as
order_idor a name longer than 25 characters, this new guard now throws instead of storing@ref:<name>.SetEventReferenceis public, and the indexing/query paths already skip names they cannot safely index, so rejecting those names at the write boundary introduces an unapproved behavioral break; preserve storage compatibility or explicitly version the API change. This is a review BLOCKER under the repository guidance.AGENTS.md reference: AGENTS.md:L67-L67
Useful? React with 👍 / 👎.