Skip to content
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

Microsoft.Extensions.Compliance.Redaction.NullRedactor doesn't seem to work #5691

Open
erwinkramer opened this issue Nov 24, 2024 · 5 comments
Assignees
Labels
area-compliance bug This issue describes a behavior which is not expected - a bug.

Comments

@erwinkramer
Copy link

Description

Please see https://github.com/erwinkramer/bank-api/blob/main/BankApi.Service/Implementation/Model.AccessLog.cs

If i change RestrictedData to UnrestrictedData, i'm expecting the property to be visible in my logging, because in https://github.com/erwinkramer/bank-api/blob/main/BankApi.Service/Defaults/Builder.Compliance.cs i set UnrestrictedData with a NullRedactor.

Reproduction Steps

  1. Clone the repo: https://github.com/erwinkramer/bank-api/tree/main
  2. Debug in Aspire mode
  3. Run the test at this line: http://{{host}}/{{apiVersion}}/teller

Expected behavior

NullRedactor should just return the value as-is, as documented.

Actual behavior

NullRedactor acts like the ErasingRedactor.

Regression?

No response

Known Workarounds

No response

Configuration

Not sure if specific to a version

.NET SDK:
Version: 9.0.100
Commit: 59db016f11
Workload version: 9.0.100-manifests.3068a692
MSBuild version: 17.12.7+5b8665660

Runtime Environment:
OS Name: Windows
OS Version: 10.0.26100
OS Platform: Windows
RID: win-x64
Base Path: C:\Program Files\dotnet\sdk\9.0.100\

Other information

No response

@erwinkramer erwinkramer added bug This issue describes a behavior which is not expected - a bug. untriaged labels Nov 24, 2024
@RussKie
Copy link
Member

RussKie commented Nov 26, 2024

/cc: @dotnet/dotnet-extensions-compliance

@amadeuszl amadeuszl self-assigned this Nov 27, 2024
@amadeuszl
Copy link
Contributor

Checking this one

@amadeuszl
Copy link
Contributor

I can reproduce the issue. Seems like this problem is related to Aspire Dashboards as they do not show any properties marked with DataClassificationAttribute for structured logs at all. When you check how structured logs are looking with eg. JsonConsole they look as they should:

{
    "EventId": 964789645,
    "LogLevel": "Information",
    "Category": "API logger",
    "Message": "Access received for user to teller data",
    "State": {
        "Message": "Microsoft.Extensions.Logging.ExtendedLogger\u002BModernTagJoiner",
        "{OriginalFormat}": "Access received for user to {dataType} data",
        "accessLog.AuthenticationType": "TEST", // no classification and only this one is visible in Aspire Dashboard
        "dataType": "teller",
        "accessLog.IShouldNotSeeIt": "", // restricted with ErasingRedactor
        "accessLog.TestField": "***", // tested with custom redactor for confidebtial
        "accessLog.UserId": "myId:accessLog.UserId" // unrestricted with NullRedactor
    }
}

How it looks in Aspire:
Image

Next, I'm going to check supported scenarios and what we can do for Aspire to make it work. We may need to create an issue on Aspire.
@eerhardt can you take a look before we go to Aspire repo?

@eerhardt
Copy link
Member

The Aspire Dashboard displays the logs that get sent to it via OTLP (open telemetry logging protocol).

It would be good to understand what is getting emitted out of the app through the OTLP exporter. Perhaps there is something in OpenTelemetry causing the problem.

cc @JamesNK

@JamesNK
Copy link
Member

JamesNK commented Nov 30, 2024

Aspire dashboard blindly displays what is sent to it. The problem must happen before the log is received by the dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-compliance bug This issue describes a behavior which is not expected - a bug.
Projects
None yet
Development

No branches or pull requests

5 participants