Skip to content

Commit

Permalink
@W-10103873: aws sdk connection pool metrics (spinnaker#21)
Browse files Browse the repository at this point in the history
* chore(dependencies): change scope of spectator-web-spring dependencies from api to implementation

since consumers of neither kork-stackdriver nor kork-web need it at compile time

Add a test to ensure that the metrics endpoint from spectator-web-spring is really available.

This paves the way to upgrade to spectator 1.0.5 where a newer version of the spring(boot)
dependencies that spectator-web-spring brings in cause problems.

See Netflix/spectator#920 for background

@W-10103873

* chore(dependences): use version 1.0.5 of spectator to stay up to date

@W-10103873

* chore(dependencies): exclude transitive dependencies of spectator-ext-aws in kork-aws

to keep spectator-ext-aws' version of the aws sdk (and the aws sdk's transitives) from
giving us newer versions of the sdk, jackson and kotlin than consumers are ready for.

@W-10103873
  • Loading branch information
dbyron-sf authored and GitHub Enterprise committed Nov 9, 2021
1 parent 2fc3b2a commit c1c172b
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 9 deletions.
8 changes: 7 additions & 1 deletion kork-aws/kork-aws.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ dependencies {
api "com.jcraft:jsch.agentproxy.jsch"
api "com.jcraft:jsch.agentproxy.connector-factory"

implementation "com.netflix.spectator:spectator-ext-aws"
implementation("com.netflix.spectator:spectator-ext-aws") {
// exclude transitives to keep spectator's version of the aws sdk from
// overriding what we specify elsewhere. It's not so much the aws sdk that
// causes problems, but its transitive dependencies -- jackson and then
// kotlin.
transitive = false
}

testImplementation "org.junit.jupiter:junit-jupiter-api"
testImplementation "org.junit.jupiter:junit-jupiter-params"
Expand Down
7 changes: 6 additions & 1 deletion kork-stackdriver/kork-stackdriver.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,14 @@ dependencies {

api "io.reactivex:rxjava"
api "com.netflix.spectator:spectator-api"
api "com.netflix.spectator:spectator-web-spring"
api "com.google.apis:google-api-services-monitoring"

implementation("com.netflix.spectator:spectator-web-spring") {
// exclude transitives since this brings in a newer version of spring (boot)
// dependencies than we're compatible with (2.2.x for spring boot and 5.2.x
// for spring as of version 1.0.5 of spectator).
transitive = false
}
implementation "org.slf4j:slf4j-api"

// Force component bringing this in to already support spring boot configure.
Expand Down
15 changes: 10 additions & 5 deletions kork-web/kork-web.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,19 @@ dependencies {
api "org.springframework.boot:spring-boot-starter-security"
api "org.springframework.security:spring-security-core"
api "com.netflix.spectator:spectator-api"
api("com.netflix.spectator:spectator-web-spring") {
// IMPORTANT: spectator-web-spring is Spring Boot 1.x; but is code compat with Boot 2. Exclude the transitives
transitive = false
}
api "com.fasterxml.jackson.core:jackson-annotations"
api "com.squareup.okhttp:okhttp"
api "com.squareup.okhttp3:okhttp"
api "com.squareup.retrofit:retrofit"

implementation "com.google.guava:guava"
implementation "javax.inject:javax.inject:1"

implementation("com.netflix.spectator:spectator-web-spring") {
// exclude transitives since this brings in a newer version of spring (boot)
// dependencies than we're compatible with (2.2.x for spring boot and 5.2.x
// for spring as of version 1.0.5 of spectator).
transitive = false
}
implementation "io.zipkin.brave:brave-instrumentation-okhttp3"

compileOnly "org.springframework.boot:spring-boot-starter-actuator"
Expand All @@ -44,3 +45,7 @@ dependencies {
testRuntimeOnly "cglib:cglib-nodep"
testRuntimeOnly "org.objenesis:objenesis"
}

test {
useJUnitPlatform()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2021 Salesforce, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.netflix.spinnaker.kork.web.config;

import static org.junit.jupiter.api.Assertions.assertEquals;

import com.netflix.spinnaker.config.MetricsEndpointConfiguration;
import java.net.URI;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.boot.web.server.LocalServerPort;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.test.context.TestPropertySource;
import org.springframework.web.util.UriComponentsBuilder;

@SpringBootTest(
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
classes = {
MetricsEndpointConfigurationTest.TestConfiguration.class,
MetricsEndpointConfiguration.class
})
@TestPropertySource(properties = {"spectator.web-endpoint.enabled = true"})
public class MetricsEndpointConfigurationTest {

@LocalServerPort int port;

@Autowired TestRestTemplate restTemplate;

@Test
public void spectatorMetricsAccess() {
URI uri =
UriComponentsBuilder.fromHttpUrl("http://localhost/spectator/metrics")
.port(port)
.build()
.toUri();

ResponseEntity<String> entity = restTemplate.getForEntity(uri, String.class);
assertEquals(HttpStatus.OK, entity.getStatusCode());
}

@SpringBootApplication
public static class TestConfiguration {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class MetricsInterceptorSpec extends Specification {
Id id = null
Timer timer = null

@Delegate
@Delegate(deprecated = true)
DefaultRegistry defaultRegistry = new DefaultRegistry()

@Override
Expand Down
2 changes: 1 addition & 1 deletion spinnaker-dependencies/spinnaker-dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ ext {
resilience4j : "1.0.0",
retrofit : "1.9.0",
retrofit2 : "2.8.1",
spectator : "0.103.0",
spectator : "1.0.5",
spek : "1.1.5",
spek2 : "2.0.9",
spring : "5.2.12.RELEASE", // this should be kept in sync with spring-boot / removed once the need for a version override is gone
Expand Down

0 comments on commit c1c172b

Please sign in to comment.