Skip to content

Commit

Permalink
Merge pull request #58 from ewiseblatt/feedback
Browse files Browse the repository at this point in the history 
Cleanup tests
  • Loading branch information
Eric Wiseblatt committed Oct 19, 2016
2 parents 0b452ac + 19ef3da commit e84bf81
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 40 deletions.
29 changes: 20 additions & 9 deletions kork-stackdriver/src/main/java/com/netflix/spectator/stackdriver/ConfigParams.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.io.FileInputStream;
import java.io.IOException;

import java.util.function.Function;
import java.util.function.Predicate;
import java.util.Collections;
import java.util.UUID;
Expand Down Expand Up @@ -67,6 +68,11 @@ public class ConfigParams {
*/
protected String instanceId;

/**
* Overridable for testing only.
*/
protected Function<String, String> determineProjectName = defaultName -> new MonitoredResourceBuilder().determineProjectName(defaultName);

/**
* Optional.
*/
Expand All @@ -82,7 +88,6 @@ public class ConfigParams {
*/
protected long counterStartTime;


/**
* Optional.
*/
Expand Down Expand Up @@ -111,15 +116,11 @@ private String validateString(String value, String purpose) {
public ConfigParams build() {
ConfigParams result = new ConfigParams();

String actualProjectName = new MonitoredResourceBuilder().determineProjectName(projectName);

result.projectName
= validateString(actualProjectName, "stackdriver projectName");
result.applicationName
= validateString(applicationName, "applicationName");
String actualProjectName = determineProjectName.apply(projectName);
result.projectName = validateString(actualProjectName, "stackdriver projectName");
result.applicationName = validateString(applicationName, "applicationName");
result.customTypeNamespace
= validateString(customTypeNamespace,
"stackdriver customTypeNamespace");
= validateString(customTypeNamespace, "stackdriver customTypeNamespace");

result.uniqueMetricsPerApplication = uniqueMetricsPerApplication;
result.counterStartTime = counterStartTime;
Expand Down Expand Up @@ -161,6 +162,16 @@ public ConfigParams build() {
return result;
}

/**
* Overrides the function for determining the actual project name
*
* This is for testing purposes only.
*/
public Builder setDetermineProjectName(Function<String, String> fn) {
determineProjectName = fn;
return this;
}

/**
* The Stackdriver namespace containing Custom Metric Descriptor types.
*
Expand Down
19 changes: 13 additions & 6 deletions kork-stackdriver/src/main/java/com/netflix/spectator/stackdriver/MetricDescriptorCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ public List<String> getRedacted() {
/**
* Constructs a hint, aliases labels list.
*/
public CustomDescriptorHint(String name,
List<String> labels) {
public CustomDescriptorHint(String name, List<String> labels) {
this(name, labels, null);
}

Expand Down Expand Up @@ -624,17 +623,25 @@ void createDescriptorInServer(Id id, Registry registry, Meter meter)
descriptor.setType(descriptorType);
descriptor.setValueType("DOUBLE");

String untransformedDescriptorType;
// The original descriptor type may have been transformed into the
// current descriptorType before this function was called.
// The hints are in terms of the original type so we need to get
// back the original type to look it up. We'll call this the
// original type.
// Currently (20160818) the case here is timers that have "statistic"
// are transformed from a single type "T" into a pair
// "T__count" and "T__totalTime". This is called with each, and for each
// we want to infer that the original type was "T" for looking up hints.
String inferredOriginalDescriptorType = descriptorType;
if (meterIsTimer(registry, meter)) {
if (id.name().endsWith("__totalTime")) {
descriptor.setUnit("ns");
}
descriptor.setMetricKind("CUMULATIVE");
int suffixOffset = descriptorType.lastIndexOf("__");
untransformedDescriptorType = descriptorType.substring(0, suffixOffset);
inferredOriginalDescriptorType = descriptorType.substring(0, suffixOffset);
} else {
descriptor.setMetricKind(meterToKind(registry, meter));
untransformedDescriptorType = descriptorType;
}

List<LabelDescriptor> labels = new ArrayList<LabelDescriptor>();
Expand All @@ -652,7 +659,7 @@ void createDescriptorInServer(Id id, Registry registry, Meter meter)
labels.add(labelDescriptor);
}

maybeAddLabelHints(untransformedDescriptorType, labels);
maybeAddLabelHints(inferredOriginalDescriptorType, labels);
if (labels.size() > 10) {
log.error("{} has too many labels for stackdriver to handle: {}",
id.name(), labels);
Expand Down
32 changes: 16 additions & 16 deletions ...tackdriver/src/test/java/com/netflix/spectator/stackdriver/MetricDescriptorCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ public long monotonicTime() {
DefaultRegistry registry = new DefaultRegistry(clock);

TestableMetricDescriptorCache cache;
String projectName = new MonitoredResourceBuilder().determineProjectName("test-project");
String projectResourceName = "projects/" + projectName;
String projectName = "test-project";
String applicationName = "test-application";

Id idA = registry.createId("idA");
Expand Down Expand Up @@ -181,6 +180,7 @@ public void setup() {
when(projectsApi.metricDescriptors()).thenReturn(descriptorsApi);

config = new ConfigParams.Builder()
.setDetermineProjectName(name -> name)
.setStackdriverStub(monitoringApi)
.setCustomTypeNamespace("TESTNAMESPACE")
.setProjectName(projectName)
Expand All @@ -204,7 +204,7 @@ public void descriptorTypeAreCompliant() {
public void descriptorNamesAreCompliant() {
// https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.metricDescriptors
Assert.assertEquals(
projectResourceName + "/metricDescriptors/" + cache.idToDescriptorType(idA),
"projects/test-project/metricDescriptors/" + cache.idToDescriptorType(idA),
cache.idToDescriptorName(idA));
}

Expand All @@ -217,15 +217,15 @@ public void testCreateDescriptorHelperCreateProperDescriptors()
= Mockito.mock(Monitoring.Projects.MetricDescriptors.Create.class);

when(descriptorsApi.create(
eq(projectResourceName), any(MetricDescriptor.class)))
eq("projects/test-project"), any(MetricDescriptor.class)))
.thenAnswer(new ReturnExecuteDescriptorArg(mockCreateMethod));
cache.createDescriptorInServer(idAXY, registry, counterA);

verify(mockCreateMethod, times(1)).execute();

ArgumentCaptor<MetricDescriptor> captor
= ArgumentCaptor.forClass(MetricDescriptor.class);
verify(descriptorsApi, times(1)).create(eq(projectResourceName),
verify(descriptorsApi, times(1)).create(eq("projects/test-project"),
captor.capture());
MetricDescriptor descriptor = captor.getValue();

Expand Down Expand Up @@ -259,13 +259,13 @@ public void testCreateDescriptorHelperRespectsHints() throws IOException {
= Mockito.mock(Monitoring.Projects.MetricDescriptors.Create.class);

when(descriptorsApi.create(
eq(projectResourceName), any(MetricDescriptor.class)))
eq("projects/test-project"), any(MetricDescriptor.class)))
.thenAnswer(new ReturnExecuteDescriptorArg(mockCreateMethod));
cache.createDescriptorInServer(idAXY, registry, counterA);

ArgumentCaptor<MetricDescriptor> captor
= ArgumentCaptor.forClass(MetricDescriptor.class);
verify(descriptorsApi, times(1)).create(eq(projectResourceName),
verify(descriptorsApi, times(1)).create(eq("projects/test-project"),
captor.capture());
verify(mockCreateMethod, times(1)).execute();

Expand All @@ -281,13 +281,13 @@ public void testCreateDescriptorHelperRespectsHints() throws IOException {
reset(mockCreateMethod);

when(descriptorsApi.create(
eq(projectResourceName), any(MetricDescriptor.class)))
eq("projects/test-project"), any(MetricDescriptor.class)))
.thenAnswer(new ReturnExecuteDescriptorArg(mockCreateMethod));
cache.createDescriptorInServer(idBXY, registry, counterB);
verify(mockCreateMethod, times(1)).execute();

captor = ArgumentCaptor.forClass(MetricDescriptor.class);
verify(descriptorsApi, times(1)).create(eq(projectResourceName),
verify(descriptorsApi, times(1)).create(eq("projects/test-project"),
captor.capture());
descriptor = captor.getValue();

Expand All @@ -313,13 +313,13 @@ public void testCreateDescriptorHelperRedacts() throws IOException {
= Mockito.mock(Monitoring.Projects.MetricDescriptors.Create.class);

when(descriptorsApi.create(
eq(projectResourceName), any(MetricDescriptor.class)))
eq("projects/test-project"), any(MetricDescriptor.class)))
.thenAnswer(new ReturnExecuteDescriptorArg(mockCreateMethod));
cache.createDescriptorInServer(idAXY, registry, counterA);

ArgumentCaptor<MetricDescriptor> captor
= ArgumentCaptor.forClass(MetricDescriptor.class);
verify(descriptorsApi, times(1)).create(eq(projectResourceName),
verify(descriptorsApi, times(1)).create(eq("projects/test-project"),
captor.capture());
verify(mockCreateMethod, times(1)).execute();

Expand Down Expand Up @@ -355,15 +355,15 @@ public void testEnsureDescriptorWhenDescriptorIsNotAlreadyLoaded()

cache.injectDescriptor(idB, new MetricDescriptor());
when(descriptorsApi.create(
eq(projectResourceName), any(MetricDescriptor.class)))
eq("projects/test-project"), any(MetricDescriptor.class)))
.thenAnswer(new ReturnExecuteDescriptorArg(mockCreateMethod));

cache.createDescriptorInServer(idA, registry, counterA);
verify(mockCreateMethod, times(1)).execute();

ArgumentCaptor<MetricDescriptor> captor
= ArgumentCaptor.forClass(MetricDescriptor.class);
verify(descriptorsApi, times(1)).create(eq(projectResourceName),
verify(descriptorsApi, times(1)).create(eq("projects/test-project"),
captor.capture());
MetricDescriptor descriptor = captor.getValue();
Assert.assertTrue(descriptor != null);
Expand All @@ -386,7 +386,7 @@ public void testEnsureDescriptorDescriptorIsAlreadyRegistered()
= new ListMetricDescriptorsResponse();
response.setMetricDescriptors(Arrays.asList(descriptorA));

when(descriptorsApi.list(projectResourceName))
when(descriptorsApi.list("projects/test-project"))
.thenReturn(mockListMethod);
when(mockListMethod.execute()).thenReturn(response);
MetricDescriptor found = cache.descriptorOrNull(
Expand All @@ -409,7 +409,7 @@ public void testEnsureDescriptorFailsIfnitFails()
= Mockito.mock(Monitoring.Projects.MetricDescriptors.List.class);
Meter counterA = registry.counter(idAXY);

when(descriptorsApi.list(projectResourceName))
when(descriptorsApi.list("projects/test-project"))
.thenThrow(new IOException());
Assert.assertTrue(
null == cache.descriptorOrNull(
Expand All @@ -422,7 +422,7 @@ public void testEnsureDescriptorFailsIfnitFails()
= new ListMetricDescriptorsResponse();
response.setMetricDescriptors(Arrays.asList(descriptorA));

when(descriptorsApi.list(projectResourceName))
when(descriptorsApi.list("projects/test-project"))
.thenReturn(mockListMethod);
when(mockListMethod.execute())
.thenReturn(response);
Expand Down
17 changes: 8 additions & 9 deletions kork-stackdriver/src/test/java/com/netflix/spectator/stackdriver/StackdriverWriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,7 @@ public long monotonicTime() {

DefaultRegistry registry = new DefaultRegistry(clock);

String projectName = new MonitoredResourceBuilder().determineProjectName("test-project");
String projectResourceName = "projects/" + projectName;

String projectName = "test-project";
String applicationName = "test-application";

Id idInternalTimerCount
Expand Down Expand Up @@ -179,6 +177,7 @@ public void setup() {
when(projectsApi.timeSeries()).thenReturn(timeseriesApi);

writerConfig = new ConfigParams.Builder()
.setDetermineProjectName(name -> name)
.setStackdriverStub(monitoringApi)
.setCustomTypeNamespace("TESTNAMESPACE")
.setProjectName(projectName)
Expand Down Expand Up @@ -416,7 +415,7 @@ public MetricDescriptor answer(InvocationOnMock invocation) {
.fetchDescriptorFromService(
timerTimeDescriptor.getName(), timerTimeDescriptor.getType());

when(timeseriesApi.create(eq(projectResourceName),
when(timeseriesApi.create(eq("projects/test-project"),
any(CreateTimeSeriesRequest.class)))
.thenReturn(mockCreateMethod);
when(mockCreateMethod.execute())
Expand All @@ -427,7 +426,7 @@ public MetricDescriptor answer(InvocationOnMock invocation) {

ArgumentCaptor<CreateTimeSeriesRequest> captor
= ArgumentCaptor.forClass(CreateTimeSeriesRequest.class);
verify(timeseriesApi, times(1)).create(eq(projectResourceName),
verify(timeseriesApi, times(1)).create(eq("projects/test-project"),
captor.capture());
// A, B, timer count and totalTime.
Assert.assertEquals(4, captor.getValue().getTimeSeries().size());
Expand Down Expand Up @@ -484,9 +483,9 @@ class MatchN implements ArgumentMatcher<CreateTimeSeriesRequest> {

MatchN match200 = new MatchN(200);
MatchN match1 = new MatchN(1);
when(timeseriesApi.create(eq(projectResourceName), argThat(match200)))
when(timeseriesApi.create(eq("projects/test-project"), argThat(match200)))
.thenReturn(mockCreateMethod);
when(timeseriesApi.create(eq(projectResourceName), argThat(match1)))
when(timeseriesApi.create(eq("projects/test-project"), argThat(match1)))
.thenReturn(mockCreateMethod);
when(mockCreateMethod.execute())
.thenReturn(null);
Expand Down Expand Up @@ -536,7 +535,7 @@ public void writeRegistryWithTimer() throws IOException {

Monitoring.Projects.MetricDescriptors.Create countMockCreateMethod
= Mockito.mock(Monitoring.Projects.MetricDescriptors.Create.class);
when(descriptorsApi.create(eq(projectResourceName),
when(descriptorsApi.create(eq("projects/test-project"),
eq(descriptorCount)))
.thenReturn(countMockCreateMethod);
doAnswer(new Answer<MetricDescriptor>() {
Expand All @@ -553,7 +552,7 @@ public MetricDescriptor answer(InvocationOnMock invocation) {

Monitoring.Projects.MetricDescriptors.Create timeMockCreateMethod
= Mockito.mock(Monitoring.Projects.MetricDescriptors.Create.class);
when(descriptorsApi.create(eq(projectResourceName),
when(descriptorsApi.create(eq("projects/test-project"),
eq(descriptorTime)))
.thenReturn(timeMockCreateMethod);
doAnswer(new Answer<MetricDescriptor>() {
Expand Down

0 comments on commit e84bf81

Please sign in to comment.