Skip to content

FC01_Feature: Audit Logging#2

Open
yasaminashoori wants to merge 13 commits intoashkanRmk:devfrom
yasaminashoori:FC01_Feature_Audit_Logging
Open

FC01_Feature: Audit Logging#2
yasaminashoori wants to merge 13 commits intoashkanRmk:devfrom
yasaminashoori:FC01_Feature_Audit_Logging

Conversation

@yasaminashoori
Copy link

@yasaminashoori yasaminashoori commented Sep 11, 2025

Audit Logging

linked issue: #1

Hi Mr. Rahmani 🌹 Thank you for the recent updates, it is great and I use FastCrud in my daily works.
I apologize for the extended development time on this implementation I wanted to ensure it follows the project's architecture patterns correctly and maintains the high quality standards of FastCrud.

Features

This PR adds audit logging functionality to FastCrud, enabling automatic tracking of all CRUD operations including Create, Update, Delete on entities.

New Endpoint for Audit

app.MapAuditLogs<AuditEntry>(
    "/api/audit-logs",
    tagName: "Audit",
    groupName: "v1"
);

Response Example

However, the responses need refinement. I want to confirm the implementation on your end, and then, if permissible, I will proceed with some refactoring.

image

🔴 Architecture and Patterns Details

1. Capture approach:

EF Core SaveChanges Interceptor and Manual Service (Manual Service for EfAuditService<TDbContext, TAuditEntry> )

public sealed class EntityAuditingInterceptor<TAuditEntry> : SaveChangesInterceptor
{
    public override InterceptionResult<int> SavingChanges()
    public override ValueTask<InterceptionResult<int>> SavingChangesAsync()
}

2. Persistence:

This code implemented with sync / async approach and audit entries will save immediately and doesn't go to the queue for processing

public override InterceptionResult<int> SavingChanges()

public override ValueTask<InterceptionResult<int>> SavingChangesAsync()

🔍 Performance and Reliability

1. Batching / Deferred

We used both in the EntityAuditingInterceptor but we don't heve neither in EfAuditService

✅EntityAuditingInterceptor

private void AddAuditEntries(DbContext? context)
{
    var auditEntries = new List<TAuditEntry>();
    
    foreach (var entry in context.ChangeTracker.Entries())
    {
        auditEntries.Add(auditEntry);
    }
    
    if (auditEntries.Any())
    {
        context.Set<TAuditEntry>().AddRange(auditEntries); 
    }
}

❌ EfAuditService

public async Task LogAsync<T>(T entity)
{
    _context.Set<TAuditEntry>().Add(auditEntry); 
    await _context.SaveChangesAsync(cancellationToken); 
}

2. Transaction coupling

✅EntityAuditingInterceptor has Same Transaction

private void AddAuditEntries(DbContext? context)
{
    context.Set<TAuditEntry>().AddRange(auditEntries);
}

❌ EfAuditService has Fire-and-forget

public async Task LogAsync<T>(T entity)
{
    _context.Set<TAuditEntry>().Add(auditEntry);
    await _context.SaveChangesAsync(cancellationToken);
}

❌ CrudServices has Fire-and-forget:

async Task<TAgg> CreateAsync(object input, CancellationToken cancellationToken)
{
    await repository.AddAsync(entity, cancellationToken);
    await repository.SaveChangesAsync(cancellationToken);
    
    if (auditService != null)
    {
        await auditService.LogAsync(entity, AuditAction.Create,);
    }
}

3. Handling large diffs (field-level vs full snapshot)

We used field-level tracking.

case EntityState.Modified:
    var modifiedProperties = entry.Properties
        .Where(p => p.IsModified && !p.Metadata.IsPrimaryKey()) 
        .ToList();
        
    auditEntry.OldValues = SerializeModifiedValues(entry, entry.OriginalValues, modifiedProperties);
    auditEntry.NewValues = SerializeModifiedValues(entry, entry.CurrentValues, modifiedProperties);
if (property.Metadata.IsForeignKey() ||
    property.Metadata.IsShadowProperty() ||
    property.Metadata.IsKey()) continue;

🔍 Configuration

1. Enable/disable globally or per module?

Global Enable exists in AddEfAuditing registration

builder.Services.AddEfAuditing<AppDbContext, AuditEntry>();

We don't have any disable globally in Configuration Options and we ONLY have per entity in IAuditEntry and HasId checks

2. DI Registration Pattern

✅ Yes

Extension method pattern
Generic constraints
coped lifetime
Factory pattern for IAuditService
Method AddCustomAuditUserProvider

public static IServiceCollection AddEfAuditing<TDbContext, TAuditEntry>(this IServiceCollection services)
{
    services.AddScoped<IAuditUserProvider, DefaultAuditUserProvider>();
    services.AddScoped<EntityAuditingInterceptor<TAuditEntry>>();
    services.AddScoped<IAuditService>(sp => new EfAuditService<x>);
    return services;
}
services.AddCustomAuditUserProvider<MyProvider>();

@yasaminashoori yasaminashoori changed the title FC01_featur: Audit Logging FC01_feature: Audit Logging Sep 11, 2025
@yasaminashoori yasaminashoori changed the title FC01_feature: Audit Logging FC01_Feature: Audit Logging Sep 11, 2025
@ashkanRmk
Copy link
Owner

Hi Yasamin,
Thank you so much for this PR and for using FastCrud in your daily work.
Really appreciate the effort and care you put into aligning with the architecture.
Audit logging is a great addition.

I’ll review it within the next few days and let you know if anything needs adjustment.

Thanks again for the contribution! 🙌

@yasaminashoori
Copy link
Author

Thanks a lot for the quick feedback🌹 I'm Glad to hear audit logging is a useful addition. Looking forward to your review and will address any adjustments you suggest.

@ashkanRmk ashkanRmk self-requested a review September 15, 2025 15:53
@ashkanRmk ashkanRmk added the enhancement New feature or request label Sep 15, 2025
@ashkanRmk
Copy link
Owner

Heads-up: I just pushed some updates to the dev branch (structural tweaks + minor service changes), so please pull/rebase from dev and resolve any merge conflicts before I start the review. That’ll keep feedback focused on the audit layer itself.

To help speed things up, could you add (or confirm) a concise section in the PR description covering:

Architecture / Pattern

Capture approach: interceptor, decorator, domain events, EF Core SaveChanges hook, or pipeline behavior?
Sync vs async persistence (any queue/outbox pattern planned)?

Performance / Reliability

Any batching or deferred write?
Transaction coupling (in same transaction vs fire-and-forget).
Handling large diffs (field-level vs full snapshot).

Configuration

How to enable/disable globally or per module.
DI registration pattern.

If you drop that summary in, the review will go much faster. Let me know once you’ve rebased and updated.
Great work so far, and thanks again!

@yasaminashoori
Copy link
Author

Thank you for the comprehensive feedback! ❤️ I'll rebase from the dev branch and resolve any conflicts, I updated the descriptions of PR and it includes all of the concerns you raised.

Looking back, I realize I should have initiated a technical discussion and maybe modeling document before implementing the audit layer to align on the architectural approach and gather your input on design decisions.

I've learned from this experience and will make sure to engage in upfront technical discussions for future significant features.
Once I've completed the rebase I'd greatly appreciate any additional feedback or suggestions you might have on the implementation approach or areas for improvement.

Thanks again for your patience and thorough guidance 🌹

@yasaminashoori
Copy link
Author

I resolved the conflicts, and I am waiting for your feedback 💯

@ashkanRmk
Copy link
Owner

ashkanRmk commented Sep 25, 2025

Hi Yasamin ✌🏻,
I’ve reviewed the code and documentation for this pull request, and I wanted to share a few considerations and suggestions that might help further improve the approach:

Column Type for OldValues and NewValues:

Using columns like OldValues and NewValues to store change snapshots is a solid idea. However, since the length of changes can be unpredictable,especially with large entities.
These columns need to be set as nvarchar(max). In practice (particularly in large companies), this can lead to performance issues and substantial memory reservation on disk, which is a concern for scalability and efficiency.

Entity-Specific Audit Endpoints:

It may be better to provide an audit log endpoint for each entity separately, rather than requiring users to specify the entity name manually. This would make the API more intuitive and easier to consume.

Configurable CRUD Audit Routes:

Just as other operations are configurable, it would be valuable to offer CRUD audit endpoints only if auditing is enabled for an entity. This way, routes are exposed conditionally, matching user configuration and improving clarity.


Overall, this is a great contribution and a strong foundation for auditing in FastCrud. Thank you again for your hard work!
Looking forward to seeing how the feature evolves.

@yasaminashoori
Copy link
Author

Hi Mr. Rahmani,
Thank you so much for taking the time to review and share such detailed feedback I really appreciate
it.🌹

I’m really sorry I haven’t had the chance to go through everything yet. It’s been a bit busy these past few days, but I’ll take some time this weekend to review everything carefully and make the updates.

Thanks again for your patience and all the helpful insights!

@yasaminashoori
Copy link
Author

yasaminashoori commented Dec 22, 2025

Thank you for your great feedback.

Summary of the last changes:

  1. Set OldValues and NewValues columns to nvarchar(max)
  2. Added entity specific audit endpoints
  3. Made audit routes configurable with includeEntities and excludeEntities parameters

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants