Skip to content

Commit

Permalink
Issue 1699 extend destination field (#1788)
Browse files Browse the repository at this point in the history
  • Loading branch information
videnkz authored Jun 23, 2021
1 parent e9b5c77 commit c751adc
Show file tree
Hide file tree
Showing 29 changed files with 563 additions and 80 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ setting `disable_instrumentations=jdbc` would disable jdbc and also enable exper
We cannot upgrade to version above 2.12.1 because this is the last version of log4j that is compatible with Java 7.
Instead, we exclude the SMTP appender (which is the vulnerable one) from our artifacts. Note that older versions of
our agent are not vulnerable as well, as the SMTP appender was never used, this is only to further reduce our users' concerns.
* Adding public APIs for setting `destination.service.resource`, `destination.address` and `destination.port` fields
for exit spans - {pull}1788[#1788]
[float]
===== Bug fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,12 @@ public void injectTraceHeaders(HeaderInjector headerInjector) {
private void doInjectTraceHeaders(MethodHandle addHeader, HeaderInjector headerInjector) {
// co.elastic.apm.agent.pluginapi.AbstractSpanInstrumentation.InjectTraceHeadersInstrumentation
}

protected void doSetDestinationAddress(@Nullable String address, int port) {
// co.elastic.apm.agent.pluginapi.AbstractSpanInstrumentation.SetDestinationAddressInstrumentation
}

protected void doSetDestinationService(@Nullable String resource) {
// co.elastic.apm.agent.pluginapi.AbstractSpanInstrumentation.SetDestinationServiceInstrumentation
}
}
12 changes: 12 additions & 0 deletions apm-agent-api/src/main/java/co/elastic/apm/api/NoopSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,16 @@ public Span setOutcome(Outcome outcome) {
public void injectTraceHeaders(HeaderInjector headerInjector) {
// noop
}

@Nonnull
@Override
public Span setDestinationAddress(@Nullable String address, int port) {
return this;
}

@Nonnull
@Override
public Span setDestinationService(@Nullable String resource) {
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,16 @@ public Transaction setOutcome(Outcome outcome) {
public void injectTraceHeaders(HeaderInjector headerInjector) {
// noop
}

@Nonnull
@Override
public Span setDestinationAddress(@Nullable String address, int port) {
return this;
}

@Nonnull
@Override
public Span setDestinationService(@Nullable String resource) {
return this;
}
}
30 changes: 30 additions & 0 deletions apm-agent-api/src/main/java/co/elastic/apm/api/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,34 @@ public interface Span {
*/
void injectTraceHeaders(HeaderInjector headerInjector);

/**
* Provides a way to manually set the destination address and port for this span. If used, values will override
* the automatically discovered ones, if there are such.
* Any value set through this method will take precedence over the automatically discovered one.
*
* NOTE: this is only relevant for spans that represent outgoing communication. Trying to invoke this method
* on a {@link Transaction} object will result with an {@link UnsupportedOperationException}.
*
* @param address a string representation of the destination host name or IP. {@code null} and empty values will
* will cause the exclusion of the address field from the span context.
* @param port the destination port. Non-positive values will cause the exclusion of the port field from the span context.
* @return this span
*/
@Nonnull
Span setDestinationAddress(@Nullable String address, int port);

/**
* Provides a way to manually set the {@code destination.service.resource} field, which is used for the construction
* of service maps and the identification of downstream services.
* Any value set through this method will take precedence over the automatically inferred one.
*
* NOTE: this is only relevant for spans that represent outgoing communication. Trying to invoke this method
* on a {@link Transaction} object will result with an {@link UnsupportedOperationException}.
*
* @param resource the string representation of the downstream service. Will be used to override automatically
* inferred value, even if {@code null}.
* @return this span
*/
@Nonnull
Span setDestinationService(@Nullable String resource);
}
15 changes: 15 additions & 0 deletions apm-agent-api/src/main/java/co/elastic/apm/api/SpanImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package co.elastic.apm.api;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* If the agent is active, it injects the implementation into this class.
Expand Down Expand Up @@ -110,4 +111,18 @@ public Span setOutcome(Outcome outcome) {
doSetOutcome(outcome);
return this;
}

@Nonnull
@Override
public Span setDestinationAddress(@Nullable String address, int port) {
doSetDestinationAddress(address, port);
return this;
}

@Nonnull
@Override
public Span setDestinationService(@Nullable String resource) {
doSetDestinationService(resource);
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package co.elastic.apm.api;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* If the agent is active, it injects the implementation from
Expand Down Expand Up @@ -158,4 +159,24 @@ public Transaction setOutcome(Outcome outcome) {
doSetOutcome(outcome);
return this;
}

/**
* @deprecated - used only for {@link co.elastic.apm.api.Span}
*/
@Nonnull
@Override
@Deprecated
public Transaction setDestinationAddress(@Nullable String address, int port) {
throw new UnsupportedOperationException();
}

/**
* @deprecated - used only for {@link co.elastic.apm.api.Span}
*/
@Nonnull
@Override
@Deprecated
public Transaction setDestinationService(@Nullable String resource) {
throw new UnsupportedOperationException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,29 @@ public class Destination implements Recyclable {
*/
private final StringBuilder address = new StringBuilder();

private boolean addressSetByUser;

/**
* The destination's port used within this context.
*/
private int port;

private boolean portSetByUser;

public Destination withAddress(@Nullable CharSequence address) {
if (address != null) {
return withAddress(address, 0, address.length());
if (address != null && !addressSetByUser) {
withAddress(address, 0, address.length());
}
return this;
}

public Destination withUserAddress(@Nullable CharSequence address) {
if (address == null || address.length() == 0) {
this.address.setLength(0);
} else {
withAddress(address, 0, address.length());
}
addressSetByUser = true;
return this;
}

Expand All @@ -52,7 +66,15 @@ public StringBuilder getAddress() {
}

public Destination withPort(int port) {
this.port = port;
if (!portSetByUser) {
this.port = port;
}
return this;
}

public Destination withUserPort(int port) {
withPort(port);
portSetByUser = true;
return this;
}

Expand All @@ -72,8 +94,10 @@ public Destination withAddressPort(@Nullable String addressPort) {
int port = parsePort(addressPort, separator + 1, addressPort.length());

if (port > 0) {
return withPort(port)
.withAddress(addressPort, 0, separator);
withPort(port);
if (!addressSetByUser) {
withAddress(addressPort, 0, separator);
}
}
}
}
Expand All @@ -96,13 +120,7 @@ private static int parsePort(CharSequence input, int start, int end) {
return port;
}

/**
* @param address address buffer
* @param start address start (inclusive)
* @param end address end (exclusive)
* @return destination with updated address
*/
private Destination withAddress(CharSequence address, int start, int end) {
private void withAddress(CharSequence address, int start, int end) {
if (address.length() > 0 && start < end) {
int startIndex = start;
int endIndex = end - 1;
Expand All @@ -121,7 +139,6 @@ private Destination withAddress(CharSequence address, int start, int end) {
this.address.append(address, startIndex, endIndex + 1);
}
}
return this;
}

/**
Expand All @@ -140,7 +157,9 @@ public boolean hasContent() {
@Override
public void resetState() {
address.setLength(0);
addressSetByUser = false;
port = 0;
portSetByUser = false;
service.resetState();
}

Expand Down Expand Up @@ -179,57 +198,83 @@ public static class Service implements Recyclable {
*/
private final StringBuilder resource = new StringBuilder();

private boolean resourceSetByUser;

/**
* Used for detecting “sameness” of services and then the display name of a service in the Service Map.
* In other words, the {@link Service#resource} is used to query data for ALL destinations. However,
* some `resources` may be nodes of the same cluster, in which case we also want to be aware.
* Eventually, we may decide to actively fetch a cluster name or similar and we could use that to detect "sameness".
* For now, for HTTP we use scheme, host, and non-default port. For anything else, we use {@code span.subtype}
* (for example- postgresql, elasticsearch).
*
* @deprecated will be removed
*/
private final StringBuilder name = new StringBuilder();

/**
* For displaying icons or similar. Currently, this should be equal to the {@code span.type}.
*
* @deprecated will be removed
*/
@Nullable
private String type;

public Service withUserResource(@Nullable String resource) {
if (resource == null || resource.isEmpty()) {
this.resource.setLength(0);
} else {
setResourceValue(resource);
}
resourceSetByUser = true;
return this;
}

public Service withResource(String resource) {
this.resource.append(resource);
if (!resourceSetByUser) {
setResourceValue(resource);
}
return this;
}

private void setResourceValue(String newValue) {
resource.setLength(0);
resource.append(newValue);
}

public StringBuilder getResource() {
return resource;
}

@Deprecated
public Service withName(String name) {
this.name.append(name);
return this;
}

@Deprecated
public StringBuilder getName() {
return name;
}

public Service withType(String type) {
this.type = type;
@Deprecated
public Service withType(@Nullable String type) {
return this;
}

@Nullable
@Deprecated
public String getType() {
return type;
}

public boolean hasContent() {
return resource.length() > 0 || name.length() > 0 || type != null;
return resource.length() > 0;
}

@Override
public void resetState() {
resource.setLength(0);
resourceSetByUser = false;
name.setLength(0);
type = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -851,25 +851,36 @@ private void serializeDestination(Destination destination) {
if (destination.hasContent()) {
writeFieldName("destination");
jw.writeByte(OBJECT_START);
if (destination.getAddress().length() > 0) {
writeField("address", destination.getAddress());
boolean hasAddress = destination.getAddress().length() > 0;
boolean hasPort = destination.getPort() > 0;
boolean hasServiceContent = destination.getService().hasContent();
if (hasAddress) {
if (hasPort || hasServiceContent) {
writeField("address", destination.getAddress());
} else {
writeLastField("address", destination.getAddress());
}
}
if (destination.getPort() > 0) {
writeField("port", destination.getPort());
if (hasPort) {
if (hasServiceContent) {
writeField("port", destination.getPort());
} else {
writeLastField("port", destination.getPort());
}
}
serializeService(destination.getService());
serializeService(hasServiceContent, destination.getService());
jw.writeByte(OBJECT_END);
jw.writeByte(COMMA);
}
}

private void serializeService(Destination.Service service) {
if (service.hasContent()) {
private void serializeService(boolean isServiceHasContent, Destination.Service service) {
if (isServiceHasContent) {
writeFieldName("service");
jw.writeByte(OBJECT_START);
writeField("name", service.getName());
writeField("resource", service.getResource());
writeLastField("type", service.getType());
writeEmptyField("name");
writeEmptyField("type");
writeLastField("resource", service.getResource());
jw.writeByte(OBJECT_END);
}
}
Expand Down Expand Up @@ -1237,6 +1248,12 @@ private void serializeUser(final User user) {
jw.writeByte(OBJECT_END);
}

void writeEmptyField(final String fieldName) {
writeFieldName(fieldName);
writeStringValue("");
jw.writeByte(COMMA);
}

void writeField(final String fieldName, final StringBuilder value) {
if (value.length() > 0) {
writeFieldName(fieldName);
Expand Down
Loading

0 comments on commit c751adc

Please sign in to comment.