Skip to content

Commit

Permalink
Sidecar tagged assignments (#13409)
Browse files Browse the repository at this point in the history
* add assignedFromTags property to assignment

* Don't fail for unknown properties on NodeDetails

This makes it easier to extend the sidecar
in the future

* Create sidecar config assignments based on tags

This is a first naive approach.
Every registration request will rebuild the assignments
and this is probably too expensive.

* fix test

* Fix more Tests

* Fine grained EtagService

Use three different caches for configs, collectors and assignments.

* Add Etag caching for UpdateRegistration results

The response to the registration PUT request contains
configuration assignments, which can be cached as well.

This requires us to invalidate the sidecar etag cache in more
situations:
 - Configration assignments
 - User triggered collector actions (stop, start, restart)

* Cleanup old assignment caching commit

* optimize

* fix after merge

* replace deprecated notempty annotation

* cache assignments per sidecar id

using just the md5 of an assignment result might
leed to sidecars missing out on updates.
An assignment is always meant for a single sidecar, caching the entire
assignment over all sidecars is wrong.

A per sidecar cache entry allows more fine grained cache invalidations.

Bump the cacheMaxSize to 5000 entries, which is a more meaningful
number, considered that this might have to hold all sidecars.

* Refactor sidecar register() call.

Only update the tagged assignments when we miss the cache.

* change cache naming

* more refactoring

* improve cache invalidation

* use fancy java

Co-authored-by: Othello Maurer <[email protected]>

* Add more cases for tag invalidation and fix tests

* add tags to Config summary

* Generate EntityTags with a better hash algorithm

As @thll noticed, Object.hashCode() is prone to having collisions

* Handle manual assignments via API

* rename

* Make tags available as config variables.

This allows configs to be written with conditionals.
E.g.:

```
<#if sidecar.tags.apache??>
 - /var/log/apache/*.log
</#if>
```

* dont lose tags on UI configuration updates

Co-authored-by: Othello Maurer <[email protected]>
  • Loading branch information
mpfz0r and thll authored Sep 19, 2022
1 parent 58a7d49 commit 920fec3
Show file tree
Hide file tree
Showing 20 changed files with 343 additions and 139 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class SidecarPluginConfiguration implements PluginConfigBean {
private Duration cacheTime = Duration.hours(1L);

@Parameter(value = PREFIX + "cache_max_size", validator = PositiveIntegerValidator.class)
private int cacheMaxSize = 100;
private int cacheMaxSize = 5000;

public Duration getCacheTime() {
return cacheTime;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.mongojack.ObjectId;

import javax.annotation.Nullable;
import java.util.Set;

@AutoValue
@WithBeanGetter
Expand All @@ -35,6 +36,7 @@ public abstract class Configuration {
public static final String FIELD_NAME = "name";
public static final String FIELD_COLOR = "color";
public static final String FIELD_TEMPLATE = "template";
public static final String FIELD_TAGS = "tags";

@Id
@ObjectId
Expand All @@ -54,30 +56,37 @@ public abstract class Configuration {
@JsonProperty(FIELD_TEMPLATE)
public abstract String template();

@JsonProperty(FIELD_TAGS)
public abstract Set<String> tags();

@JsonCreator
public static Configuration create(@JsonProperty(FIELD_ID) String id,
@JsonProperty(FIELD_COLLECTOR_ID) String collectorId,
@JsonProperty(FIELD_NAME) String name,
@JsonProperty(FIELD_COLOR) String color,
@JsonProperty(FIELD_TEMPLATE) String template) {
@JsonProperty(FIELD_TEMPLATE) String template,
@JsonProperty(FIELD_TAGS) @Nullable Set<String> tags) {
return builder()
.id(id)
.collectorId(collectorId)
.name(name)
.color(color)
.template(template)
.tags(tags == null ? Set.of() : tags)
.build();
}

public static Configuration create(String collectorId,
String name,
String color,
String template) {
public static Configuration createWithoutId(String collectorId,
String name,
String color,
String template,
Set<String> tags) {
return create(new org.bson.types.ObjectId().toHexString(),
collectorId,
name,
color,
template);
template,
tags);
}

public static Builder builder() {
Expand All @@ -98,6 +107,8 @@ public abstract static class Builder {

public abstract Builder template(String template);

public abstract Builder tags(Set<String> tags);

public abstract Configuration build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.mongojack.Id;
import org.mongojack.ObjectId;

import java.util.Set;

@AutoValue
public abstract class ConfigurationSummary {
@JsonProperty("id")
Expand All @@ -38,20 +40,25 @@ public abstract class ConfigurationSummary {
@JsonProperty("color")
public abstract String color();

@JsonProperty("tags")
public abstract Set<String> tags();

@JsonCreator
public static ConfigurationSummary create(@JsonProperty("id") @Id @ObjectId String id,
@JsonProperty("name") String name,
@JsonProperty("collector_id") String collectorId,
@JsonProperty("color") String color) {
return new AutoValue_ConfigurationSummary(id, name, collectorId, color);
@JsonProperty("color") String color,
@JsonProperty("tags") Set<String> tags) {
return new AutoValue_ConfigurationSummary(id, name, collectorId, color, tags);
}

public static ConfigurationSummary create(Configuration configuration) {
return create(
configuration.id(),
configuration.name(),
configuration.collectorId(),
configuration.color());
configuration.color(),
configuration.tags());
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.auto.value.AutoValue;

Expand All @@ -29,6 +30,7 @@

@AutoValue
@JsonAutoDetect
@JsonIgnoreProperties(ignoreUnknown = true)
public abstract class NodeDetails {
@JsonProperty("operating_system")
@NotNull
Expand All @@ -52,7 +54,6 @@ public abstract class NodeDetails {
public abstract CollectorStatusList statusList();

@JsonProperty("tags")
@Nullable
public abstract Set<String> tags();

@JsonProperty("collector_configuration_directory")
Expand All @@ -67,6 +68,6 @@ public static NodeDetails create(@JsonProperty("operating_system") String operat
@JsonProperty("status") @Nullable CollectorStatusList statusList,
@JsonProperty("tags") @Nullable Set<String> tags,
@JsonProperty("collector_configuration_directory") @Nullable String configDir) {
return new AutoValue_NodeDetails(operatingSystem, ip, metrics, logFileList, statusList, tags, configDir);
return new AutoValue_NodeDetails(operatingSystem, ip, metrics, logFileList, statusList, tags == null ? Set.of() : tags, configDir);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public static Status fromString(String statusString) {
public abstract NodeDetails nodeDetails();

@JsonProperty
@Nullable
public abstract List<ConfigurationAssignment> assignments();

@JsonProperty
Expand Down Expand Up @@ -133,7 +132,7 @@ public static Sidecar create(@JsonProperty(FIELD_ID) @Id @ObjectId String id,
.nodeId(nodeId)
.nodeName(nodeName)
.nodeDetails(nodeDetails)
.assignments(assignments)
.assignments(assignments == null ? List.of() : assignments)
.sidecarVersion(sidecarVersion)
.lastSeen(lastSeen)
.build();
Expand All @@ -151,6 +150,7 @@ public static Sidecar create(@JsonProperty(FIELD_NODE_ID) String nodeId,
.nodeDetails(nodeDetails)
.sidecarVersion(sidecarVersion)
.lastSeen(DateTime.now(DateTimeZone.UTC))
.assignments(List.of())
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.auto.value.AutoValue;

import javax.annotation.Nullable;
import java.util.Set;

@AutoValue
@JsonAutoDetect
public abstract class ConfigurationAssignment {
Expand All @@ -30,9 +33,13 @@ public abstract class ConfigurationAssignment {
@JsonProperty
public abstract String configurationId();

@JsonProperty
public abstract Set<String> assignedFromTags();

@JsonCreator
public static ConfigurationAssignment create(@JsonProperty("collector_id") String collectorId,
@JsonProperty("configuration_id") String configurationId) {
return new AutoValue_ConfigurationAssignment(collectorId, configurationId);
@JsonProperty("configuration_id") String configurationId,
@JsonProperty("assigned_from_tags") @Nullable Set<String> assignedFromTags) {
return new AutoValue_ConfigurationAssignment(collectorId, configurationId, assignedFromTags == null ? Set.of() : assignedFromTags);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.graylog.plugins.sidecar.services.ActionService;
import org.graylog.plugins.sidecar.services.CollectorService;
import org.graylog.plugins.sidecar.services.ConfigurationService;
import org.graylog.plugins.sidecar.services.EtagService;
import org.graylog.plugins.sidecar.services.SidecarService;
import org.graylog.plugins.sidecar.system.SidecarConfiguration;
import org.graylog2.audit.jersey.AuditEvent;
Expand Down Expand Up @@ -90,6 +91,7 @@ public class AdministrationResource extends RestResource implements PluginRestRe
private final AdministrationFiltersFactory administrationFiltersFactory;
private final ActiveSidecarFilter activeSidecarFilter;
private final SidecarStatusMapper sidecarStatusMapper;
private final EtagService etagService;

@Inject
public AdministrationResource(SidecarService sidecarService,
Expand All @@ -98,14 +100,16 @@ public AdministrationResource(SidecarService sidecarService,
ActionService actionService,
AdministrationFiltersFactory administrationFiltersFactory,
ClusterConfigService clusterConfigService,
SidecarStatusMapper sidecarStatusMapper) {
SidecarStatusMapper sidecarStatusMapper,
EtagService etagService) {
final SidecarConfiguration sidecarConfiguration = clusterConfigService.getOrDefault(SidecarConfiguration.class, SidecarConfiguration.defaultConfiguration());
this.sidecarService = sidecarService;
this.configurationService = configurationService;
this.collectorService = collectorService;
this.actionService = actionService;
this.administrationFiltersFactory = administrationFiltersFactory;
this.sidecarStatusMapper = sidecarStatusMapper;
this.etagService = etagService;
this.activeSidecarFilter = new ActiveSidecarFilter(sidecarConfiguration.sidecarInactiveThreshold());
this.searchQueryParser = new SearchQueryParser(Sidecar.FIELD_NODE_NAME, SidecarResource.SEARCH_FIELD_MAPPING);
}
Expand Down Expand Up @@ -166,6 +170,7 @@ public Response setAction(@ApiParam(name = "JSON body", required = true)
.collect(Collectors.toList());
final CollectorActions collectorActions = actionService.fromRequest(bulkActionRequest.sidecarId(), actions);
actionService.saveAction(collectorActions);
etagService.invalidateRegistration(bulkActionRequest.sidecarId());
}

return Response.accepted().build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
package org.graylog.plugins.sidecar.rest.resources;

import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.hash.Hashing;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
Expand Down Expand Up @@ -136,15 +136,15 @@ public Collector getCollector(@ApiParam(name = "id", required = true)
@RequiresPermissions(SidecarRestPermissions.COLLECTORS_READ)
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "List all collectors")
public Response listCollectors(@Context HttpHeaders httpHeaders) {
public Response listCollectors(@Context HttpHeaders httpHeaders) throws JsonProcessingException {
String ifNoneMatch = httpHeaders.getHeaderString("If-None-Match");
Boolean etagCached = false;
Response.ResponseBuilder builder = Response.noContent();

// check if client is up to date with a known valid etag
// check if client is up-to-date with a known valid etag
if (ifNoneMatch != null) {
EntityTag etag = new EntityTag(ifNoneMatch.replaceAll("\"", ""));
if (etagService.isPresent(etag.toString())) {
if (etagService.collectorsAreCached(etag.toString())) {
etagCached = true;
builder = Response.notModified();
builder.tag(etag);
Expand All @@ -157,12 +157,10 @@ public Response listCollectors(@Context HttpHeaders httpHeaders) {
CollectorListResponse collectorListResponse = CollectorListResponse.create(result.size(), result);

// add new etag to cache
String etagString = collectorsToEtag(collectorListResponse);

EntityTag collectorsEtag = new EntityTag(etagString);
EntityTag collectorsEtag = etagService.buildEntityTagForResponse(collectorListResponse);
builder = Response.ok(collectorListResponse);
builder.tag(collectorsEtag);
etagService.put(collectorsEtag.toString());
etagService.registerCollector(collectorsEtag.toString());
}

// set cache control
Expand Down Expand Up @@ -211,7 +209,7 @@ public Response createCollector(@ApiParam(name = "JSON body", required = true)
if (validationResult.failed()) {
return Response.status(Response.Status.BAD_REQUEST).entity(validationResult).build();
}
etagService.invalidateAll();
etagService.invalidateAllCollectors();
return Response.ok().entity(collectorService.save(collector)).build();
}

Expand All @@ -230,7 +228,7 @@ public Response updateCollector(@ApiParam(name = "id", required = true)
if (validationResult.failed()) {
return Response.status(Response.Status.BAD_REQUEST).entity(validationResult).build();
}
etagService.invalidateAll();
etagService.invalidateAllCollectors();
return Response.ok().entity(collectorService.save(collector)).build();
}

Expand All @@ -248,8 +246,8 @@ public Response copyCollector(@ApiParam(name = "id", required = true)
if (validationResult.failed()) {
return Response.status(Response.Status.BAD_REQUEST).entity(validationResult).build();
}
etagService.invalidateAll();
collectorService.save(collector);
etagService.invalidateAllCollectors();
return Response.accepted().build();
}

Expand All @@ -272,7 +270,7 @@ public Response deleteCollector(@ApiParam(name = "id", required = true)
if (deleted == 0) {
return Response.notModified().build();
}
etagService.invalidateAll();
etagService.invalidateAllCollectors();
return Response.accepted().build();
}

Expand Down Expand Up @@ -351,9 +349,4 @@ private boolean validatePath(String path) {
return VALID_PATH_PATTERN.matcher(path).matches();
}

private String collectorsToEtag(CollectorListResponse collectors) {
return Hashing.md5()
.hashInt(collectors.hashCode()) // avoid negative values
.toString();
}
}
Loading

0 comments on commit 920fec3

Please sign in to comment.