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

[BUG] Serialization of nested properties is broken #205

Closed
andreycha opened this issue Aug 30, 2022 · 2 comments
Closed

[BUG] Serialization of nested properties is broken #205

andreycha opened this issue Aug 30, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@andreycha
Copy link
Contributor

andreycha commented Aug 30, 2022

ECS integration/library project(s) (e.g. Elastic.CommonSchema.Serilog): Elastic.CommonSchema.Serilog

ECS schema version (e.g. 1.4.0): 8.3.1

ECS .NET assembly version (e.g. 1.4.2): main branch

Description of the problem, including expected versus actual behavior:
Iran Serilog ASP.NET Core example in main branch and noticed that nested fields have wrong names after serialization, e.g. check http and user_agent fields in this example (document is reduced for brevity):

{
    "@timestamp": "2022-08-30T16:23:12.7722738+02:00",
    "log.level": "Information",
    "message": "Request finished HTTP/2 GET https://localhost:59987/weatherforecast - - - 200 - application/json;+charset=utf-8 423.9797ms",
    "ecs":
    {
        "version": "v8.3.1"
    },
    "http":
    {
        "request_method": "GET", // should be "request.method": "GET"
        "response_status_code": 200 // should be "response.status_code": "200"
    },
    "log":
    {
        "level": "Information",
        "logger": "Microsoft.AspNetCore.Hosting.Diagnostics"
    },
    "user_agent":
    {
        "os":
        {
            "family": "Windows",
            "full": "Windows 10",
            "version": "10"
        },
        "device_name": "Other", // should be "device.name": "Other"
        "name": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.5112.102 Safari/537.36 OPR/90.0.4480.54",
        "original": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.5112.102 Safari/537.36 OPR/90.0.4480.54",
        "version": "90.0.4480"
    }
}

PR #194 has introduced new definition of ECS document where field names are defined using DataMember attribute. The problem is that it's not supported by System.Text.Json serializer. There is an own JsonPropertyName attribute for that purpose.

Tests in Elastic.CommonSchema.Tests.Serializes are not doing enough checks, e.g. field Name2 with overridden name here is not verified.

This is critical, since it affects all the .NET integrations and breaks log shipping.

Steps to reproduce:

  1. Run Serilog ASP.NET Core application
  2. Check console output
@andreycha andreycha added the bug Something isn't working label Aug 30, 2022
@Mpdreamz
Copy link
Member

Argh what an oversight on our end! Not the first time dotnet/runtime#29975 bites me in the rear either!

Thanks for raising this @andreycha I've opened #208 to address this.

@Mpdreamz
Copy link
Member

Mpdreamz commented Sep 1, 2022

We needed in fact both DataMember and JsonPropertyName to ensure compatibility with older Elasticsearch clients that still use Json.NET.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants