Support more ECS service fields

[szisiu]

Currently, only service name is supported.

It'd be beneficial to add more service fields from ECS, eg.: id, version, type. So when inspecting logs in kibana we could see not only the service name but also the service version, etc.

New fields should be passed through config files or directly in the code (which I am interested in):

RollingFileAppender.newBuilder()
                    .setName("ecs_file_appender")
                    .setLayout(EcsLayout.newBuilder()
                            .setServiceName("my_service_name")
                            .setServiceVersion("my_service_version")
                            // more service config options
                            .build())
                     // more config options
                    .build();

Refs:

• https://www.elastic.co/guide/en/ecs/current/ecs-service.html
• https://www.elastic.co/guide/en/ecs-logging/java/1.x/index.html

[felixbarny]

You can do that using the setAdditionalFields method.

See these examples from the tests:

ecs-logging-java/log4j2-ecs-layout/src/test/java/co/elastic/logging/log4j2/Log4j2EcsLayoutTest.java

Lines 73 to 82 in 09c07cf

	.setAdditionalFields(new KeyValuePair[]{
	new KeyValuePair("cluster.uuid", "9fe9134b-20b0-465e-acf9-8cc09ac9053b"),
	new KeyValuePair("node.id", "${node.id}"),
	new KeyValuePair("empty", "${empty}"),
	new KeyValuePair("clazz", "%C"),
	new KeyValuePair("custom", "%custom"),
	new KeyValuePair("emptyPattern", "%notEmpty{%invalidPattern}"),
	new KeyValuePair("key1", "value1"),
	new KeyValuePair("key2", "value2"),
	})

ecs-logging-java/log4j2-ecs-layout/src/test/resources/log4j2-test.xml

Lines 8 to 17 in 09c07cf

	<EcsLayout serviceName="test" includeMarkers="true" includeOrigin="true" eventDataset="testdataset.log">
	<KeyValuePair key="cluster.uuid" value="9fe9134b-20b0-465e-acf9-8cc09ac9053b"/>
	<KeyValuePair key="node.id" value="${node.id}"/>
	<KeyValuePair key="empty" value="${empty}"/>
	<KeyValuePair key="clazz" value="%C"/>
	<KeyValuePair key="custom" value="%custom"/>
	<KeyValuePair key="emptyPattern" value="%notEmpty{%invalidPattern}"/>
	<KeyValuePair key="key1" value="value1"/>
	<KeyValuePair key="key2" value="value2"/>
	</EcsLayout>

[szisiu]

setAdditionalFields works. I was thinking that we could make it more elegant and unified:

RollingFileAppender.newBuilder()
                    .setName("ecs_file_appender")
                    .setLayout(EcsLayout.newBuilder()
                            .setConfiguration(config)
                            .setServiceName("my_service_name")
                            .setServiceVersion("my_service_version")
                            .build());

instead of:

RollingFileAppender.newBuilder()
                    .setName("ecs_file_appender")
                    .setLayout(EcsLayout.newBuilder()
                            .setConfiguration(config)
                            .setServiceName("my_service_name")
                            .setAdditionalFields(new KeyValuePair[] { new KeyValuePair("service.version", "my_service_version" })
                            .build());

Thanks.

[felixbarny]

I see. I'm just a bit worried about the potential explosion of fields that need to be supported and kept up-to-date with ECS.

What about adding a method that's a bit more convenient, such as

.addAdditionalField("service.version", "my_service_version" )

[szisiu]

I'm just a bit worried about the potential explosion of fields that need to be supported and kept up-to-date with ECS.

ok, I see your point now. maybe you can just support a service.version to have a basic context of application name and version (easy to use and discover using the builder api or in config files)? imo, these two fileds are a 'must-have' when dealing wih logs or metrics (I had a case recenlty when I needed to figure out what app version caused some issues - and inspecting logs).

What about adding a method that's a bit more convenient

this looks ok as well

[felixbarny]

Adding service.version sounds fine to me.

I've created elastic/ecs-logging#58 to add this to the common ECS logging spec.

Are you interested in creating a PR for ecs-logging-java?

[szisiu]

I will review the project and will try to add the field.

[szisiu]

Hi @felixbarny, I have prepared a branch (from master) but have some problems with pushing it:

remote: Permission to elastic/ecs-logging-java.git denied.
fatal: unable to access 'https://github.com/elastic/ecs-logging-java.git/': The requested URL returned error: 403

I use IntelliJ 2021.1.2, enabled GitHub integration and logged in.

It has been a while since my last contribution in GitHub. Am I doing sth wrong?

[xeraa]

Without write access to the repository, you need to fork and send the PR from your branch in the fork: https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork :)

[szisiu]

Understood and PR created. Thanks. Waiting for feedback.

[felixbarny]

We have recently added support for two more service fields:

• Added support for service.version #168
• Add support for service.environment #184