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

Add xds client metric for receiving invalid resource #1005

Merged
merged 1 commit into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ and what APIs have changed, if applicable.

## [Unreleased]

## [29.57.0] - 2024-06-16
- Add xds client metric for receiving invalid resource

## [29.56.1] - 2024-06-06
- prevent duplicate uri property update

Expand Down Expand Up @@ -5698,7 +5701,8 @@ patch operations can re-use these classes for generating patch messages.

## [0.14.1]

[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.56.1...master
[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.57.0...master
[29.57.0]: https://github.com/linkedin/rest.li/compare/v29.56.1...v29.57.0
[29.56.1]: https://github.com/linkedin/rest.li/compare/v29.56.0...v29.56.1
[29.56.0]: https://github.com/linkedin/rest.li/compare/v29.55.0...v29.56.0
[29.55.0]: https://github.com/linkedin/rest.li/compare/v29.54.0...v29.55.0
Expand Down
12 changes: 12 additions & 0 deletions d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmx.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class XdsClientJmx implements XdsClientJmxMBean

private final AtomicBoolean _isConnected = new AtomicBoolean();
private final AtomicInteger _resourceNotFoundCount = new AtomicInteger();
private final AtomicInteger _resourceInvalidCount = new AtomicInteger();
private final XdsServerMetricsProvider _xdsServerMetricsProvider;

@Deprecated
Expand Down Expand Up @@ -67,6 +68,12 @@ public int getResourceNotFoundCount()
return _resourceNotFoundCount.get();
}

@Override
public int getResourceInvalidCount()
{
return _resourceInvalidCount.get();
}

@Override
public long getXdsServerLatencyMin() {
return _xdsServerMetricsProvider.getLatencyMin();
Expand Down Expand Up @@ -130,4 +137,9 @@ public void incrementResourceNotFoundCount()
{
_resourceNotFoundCount.incrementAndGet();
}

public void incrementResourceInvalidCount()
{
_resourceInvalidCount.incrementAndGet();
}
}
3 changes: 3 additions & 0 deletions d2/src/main/java/com/linkedin/d2/jmx/XdsClientJmxMBean.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public interface XdsClientJmxMBean {
// when the resource is not found.
int getResourceNotFoundCount();

// when the resource is invalid.
int getResourceInvalidCount();

/**
* Get minimum of Xds server latency, which is from when the resource is updated on the Xds server to when the
* client receives it.
Expand Down
22 changes: 20 additions & 2 deletions d2/src/main/java/com/linkedin/d2/xds/XdsClientImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.linkedin.d2.jmx.NoOpXdsServerMetricsProvider;
import com.linkedin.d2.jmx.XdsClientJmx;
import com.linkedin.d2.xds.GlobCollectionUtils.D2UriIdentifier;
import com.linkedin.util.RateLimitedLogger;
import com.linkedin.util.clock.SystemClock;
import indis.XdsD2;
import io.envoyproxy.envoy.service.discovery.v3.AggregatedDiscoveryServiceGrpc;
Expand Down Expand Up @@ -68,6 +69,8 @@
public class XdsClientImpl extends XdsClient
{
private static final Logger _log = LoggerFactory.getLogger(XdsClientImpl.class);
private static final RateLimitedLogger RATE_LIMITED_LOGGER =
new RateLimitedLogger(_log, TimeUnit.MINUTES.toMillis(10), SystemClock.instance());
public static final long DEFAULT_READY_TIMEOUT_MILLIS = 2000L;

private final Map<ResourceType, Map<String, ResourceSubscriber>> _resourceSubscribers = Maps.immutableEnumMap(
Expand Down Expand Up @@ -136,7 +139,7 @@ void watchXdsResource(String resourceName, ResourceWatcher watcher)
ResourceSubscriber subscriber = resourceSubscriberMap.get(resourceName);
if (subscriber == null)
{
subscriber = new ResourceSubscriber(watcher.getType(), resourceName);
subscriber = new ResourceSubscriber(watcher.getType(), resourceName, _xdsClientJmx);
resourceSubscriberMap.put(resourceName, subscriber);
ResourceType type;
String adjustedResourceName;
Expand Down Expand Up @@ -506,6 +509,7 @@ static class ResourceSubscriber
private final ResourceType _type;
private final String _resource;
private final Set<ResourceWatcher> _watchers = new HashSet<>();
private final XdsClientJmx _xdsClientJmx;
@Nullable
private ResourceUpdate _data;

Expand All @@ -522,10 +526,11 @@ public void setData(@Nullable ResourceUpdate data)
_data = data;
}

ResourceSubscriber(ResourceType type, String resource)
ResourceSubscriber(ResourceType type, String resource, XdsClientJmx xdsClientJmx)
{
_type = type;
_resource = resource;
_xdsClientJmx = xdsClientJmx;
}

void addWatcher(ResourceWatcher watcher)
Expand Down Expand Up @@ -555,6 +560,19 @@ private void onData(ResourceUpdate data, XdsServerMetricsProvider metricsProvide
trackServerLatency(data, metricsProvider); // data updated, track xds server latency
_data = data;
}
else
{
if (_type == ResourceType.D2_URI_MAP || _type == ResourceType.D2_URI)
{
RATE_LIMITED_LOGGER.warn("Received invalid data for {} {}, data: {}", _type, _resource, data);
}
else
{
_log.warn("Received invalid data for {} {}, data: {}", _type, _resource, data);
}
_xdsClientJmx.incrementResourceInvalidCount();
}

if (_data == null)
{
_log.info("Initializing {} {} to empty data.", _type, _resource);
Expand Down
6 changes: 4 additions & 2 deletions d2/src/test/java/com/linkedin/d2/xds/TestXdsClientImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -447,8 +447,8 @@ private static class XdsClientImplFixture
XdsClientImpl _xdsClientImpl;
@Mock
XdsClientJmx _xdsClientJmx;
ResourceSubscriber _nodeSubscriber = spy(new ResourceSubscriber(NODE, SERVICE_RESOURCE_NAME));
ResourceSubscriber _clusterSubscriber = spy(new ResourceSubscriber(D2_URI_MAP, CLUSTER_RESOURCE_NAME));
ResourceSubscriber _nodeSubscriber;
ResourceSubscriber _clusterSubscriber;
Map<ResourceType, Map<String, ResourceSubscriber>> _subscribers = new HashMap<>();
@Mock
XdsClient.ResourceWatcher _resourceWatcher;
Expand All @@ -463,6 +463,8 @@ private static class XdsClientImplFixture
XdsClientImplFixture(boolean useGlobCollections)
{
MockitoAnnotations.initMocks(this);
_nodeSubscriber = spy(new ResourceSubscriber(NODE, SERVICE_RESOURCE_NAME, _xdsClientJmx));
_clusterSubscriber = spy(new ResourceSubscriber(D2_URI_MAP, CLUSTER_RESOURCE_NAME, _xdsClientJmx));

doNothing().when(_resourceWatcher).onChanged(any());
for (ResourceSubscriber subscriber : Lists.newArrayList(_nodeSubscriber, _clusterSubscriber))
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
version=29.56.1
version=29.57.0
group=com.linkedin.pegasus
org.gradle.configureondemand=true
org.gradle.parallel=true
Expand Down
Loading