Provide a way to customize default serviceName in RequestLog

[ikhoon]

The default serviceName of a service is an FQCN of the innermost class.
And a serviceName could get overridden by ctx.logBuilder().name(serviceName, methodName).
Sometimes, users want to globally change the serviceName convention to a simple name of the innermost class.

We might provide an option to ServerBuilder such as:

Server.builder()
     .serviceNaming(serviceName -> {
          var lastDotIndex = serviceName.lastIndexOf('.');
          if (lastDotIndex > -1) {
              return serviceName.substring(lastDotIndex, serviceName.length());
          }
          ...
     });

And we can also provide a built-in abbreviation algorithm like logback's Conversion Word.

ServiceNameRule.shorten(15);

[hexoul]

For me, it should be useful for sure. IMHO, not only abbreviation algorithm but also other rules seems good to be flags. For instance,

1. ServiceNamingRule.dot.last(2) to pick last two concatenation separated by dot.
2. ServiceNamingRule.simple to pick just class name.
3. ServiceNamingRule.shorten(15)

[ikhoon]

That's a nice suggestion. Are you interested in sending a PR? 😉

[hexoul]

I hope so. 🤣 I am curious about the preference for this of you. If the user invokes ServerBuilder#serviceNaming and injects MeterIdPrefixFunction, should it be combinable? how can we deal with?

https://github.com/line/armeria/blob/master/spring/boot2-autoconfigure/src/main/java/com/linecorp/armeria/spring/AbstractArmeriaAutoConfiguration.java#L74-L76

[ikhoon]

I think ServerBuilder.serviceNaming could be applied to RequestLog.serviceName if a user does not specify serviceName explicitly.
MeterIdPrefixFunction will use RequestLog.serviceName itself without any modifications.

[ikhoon]

The ServerBuilder#serviceNaming will customize the populated newServiceName in the following code.

armeria/core/src/main/java/com/linecorp/armeria/common/logging/DefaultRequestLog.java

Lines 1050 to 1068 in 8689584

	// Set serviceName from ServiceType or innermost class name
	if (newServiceName == null) {
	if (rpcReq != null) {
	final String serviceType = rpcReq.serviceType().getName();
	if ("com.linecorp.armeria.internal.common.grpc.GrpcLogUtil".equals(serviceType)) {
	// Parse gRPC serviceName and methodName
	final String fullMethodName = rpcReq.method();
	final int methodIndex = fullMethodName.lastIndexOf('/');
	newServiceName = fullMethodName.substring(0, methodIndex);
	if (newName == null) {
	newName = fullMethodName.substring(methodIndex + 1);
	}
	} else {
	newServiceName = serviceType;
	}
	} else if (config != null) {
	newServiceName = getInnermostServiceName(config.service());
	}
	}

[hexoul]

Thanks for the references! Let me take a look more in considering that way.

[hexoul]

Before PR, I would like to share the plan first. First of all, I consider to support serviceNaming(Function) without ServiceNameRule and improve later.

For me, the config is supposed to be delivered from ServiceConfig and used under DefaultRequestLog you referred.
In other words, I will propose

• AbstractServiceBindingBuilder#serviceNaming
• ServiceBindingBuilder#serviceNaming
• ServiceConfigSetters#serviceNaming
• DefaultServiceConfigSetters#serviceNaming

and call like serviceName = config.serviceNaming().apply(newServiceName) instead of serviceName = newServiceName in DefaultRequestLog#setNamesIfAbsent.

If you mind something, please give me an opinion.

[ikhoon]

As you know, users can customize the default serviceName using ServiceBindingBuilder.defaultServiceName().
That means we don't need to serviceNaming rule at a service level.
Is it better to only apply to a server level by adding ServerBuilder.serviceNaming(ServiceNaming serviceNaming)?

and call like serviceName = config.serviceNaming().apply(newServiceName) instead of serviceName = newServiceName in DefaultRequestLog#setNamesIfAbsent.

Then, this should be ctx.config().server().config().serviceNaming().apply(...).

[hexoul]

users can customize the default serviceName using ServiceBindingBuilder.defaultServiceName().

I also thought the case that a user uses serviceNaming without using defaultServiceName. It was because I consider serviceNaming as a post processor. For example, even if users set a default service name to blahblah and the function which picks first two letters is given, the service name become bl finally. IMHO, it will be more useful with logback's shorten algorithm.

If we only apply the rule in server level not service level as you said, is it better to name ServerBuilder#defaultServiceNaming instead of just serviceNaming?

In short, it depends on needs. WDYT?

[ikhoon]

For me, it should be useful for sure. IMHO, not only abbreviation algorithm but also other rules seems good to be flags. For instance,

1. `ServiceNamingRule.dot.last(2)` to pick last two concatenation separated by dot.

2. `ServiceNamingRule.simple` to pick just class name.

3. `ServiceNamingRule.shorten(15)`

You don't have to implement all of these implementations at once. You might want to start by designing an API and add more implementations later.

[ikhoon]

I also thought the case that a user uses serviceNaming without using defaultServiceName. It was because I consider serviceNaming as a post processor. For example, even if users set a default service name to blahblah and the function which picks first two letters is given, the service name become bl finally. IMHO, it will be more useful with logback's shorten algorithm.

A serviceNaming function could be applied to a defaultServiceName.
However, I am wondering if users need a serviceNaming rule for a specific service.
Because if a defaulServiceName is set, users already knows when serviceNaming(...) is configured.

Server.builder()
      .route()
      .defaultServiceName("foobar")
      .serviceNaming(name -> name.substring(...))
      .build(new MyService())

Do you have cases that such sophisticated serviceNaming rules are required? Otherwise, I prefer ServerBuilder.serviceNaming() to give simplicity and reduce confusion. :-)

[hexoul]

ServerBuilder.serviceNaming()

I see. It seems that I unnecessarily worried layered serviceNaming. Agreed that the suggestion makes the problem simple and reduce confusion. Let me try with that way. Thanks. :)

[hexoul]

Can we close this issue? @ikhoon

[ikhoon]

Nice catch! This was fixed by #3453.