Skip to content

Conversation

@felixbarny
Copy link
Member

@felixbarny felixbarny commented Jul 26, 2019

rather than the name of the generated servlet
closes #429
depends on #748


public T appendToName(String s, int priority) {
if (priority >= namePriority) {
name.append(s);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name.append(s);
name.append(s);
this.namePriority = priority;

Add a test that fails without that

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

@DisabledOnOs(OS.MAC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 Bravo!!

transaction = tracer.startTransaction(TraceContext.asRoot(), null, clazz.getClassLoader())
.withName(transactionName.isEmpty() ? signature : transactionName)
.withName(transactionName, AbstractSpan.PRIO_USER_SUPPLIED)
.withName(signature, AbstractSpan.PRIO_METHOD_SIGNATURE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if/else reads better..

@eyalkoren
Copy link
Contributor

I merged a PR disabling the annoying JMS test that failed, so you can merge with master..

rather than the name of the generated servlet closes elastic#429
@felixbarny felixbarny force-pushed the jsp-transaction-name branch from 2845973 to d53011a Compare July 29, 2019 13:43
@felixbarny felixbarny merged commit 10ee220 into elastic:master Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants