refactor(retrofit2): addressed review comments

echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy

@@ -97,6 +97,7 @@ class InteractiveNotificationCallbackHandler {
 
     // TODO(lfp): error handling (retries?). I'd like to respond to the message in a thread, but
     //  have been unable to make that work. Troubleshooting with Slack support.
+    // TODO(lfp): need to retrieve user's accounts to pass in X-SPINNAKER-ACCOUNTS
     final ResponseBody response = Retrofit2SyncCall.execute(spinnakerService.notificationCallback(callback, callback.getUser()));
     log.debug("Received callback response from downstream Spinnaker service: " + response.string());
 

echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy

@@ -19,6 +19,7 @@ package com.netflix.spinnaker.echo.pagerduty
 
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.netflix.spinnaker.echo.api.Notification
+import com.netflix.spinnaker.echo.config.PagerDutyConfigurationProperties
 import com.netflix.spinnaker.echo.controller.EchoResponse
 import com.netflix.spinnaker.echo.notification.NotificationService
 import com.netflix.spinnaker.echo.services.Front50Service
@@ -47,8 +48,8 @@ class PagerDutyNotificationService implements NotificationService {
   @Autowired
   PagerDutyService pagerDuty
 
-  @Value('${pager-duty.token:}')
-  String token
+  @Autowired
+  PagerDutyConfigurationProperties pagerDutyConfigurationProperties
 
   @Autowired
   Front50Service front50Service
@@ -66,7 +67,7 @@ class PagerDutyNotificationService implements NotificationService {
     notification.to.each { serviceKey ->
       try {
         Map response = Retrofit2SyncCall.execute(pagerDuty.createEvent(
-          "Token token=${token}",
+          "Token token=${pagerDutyConfigurationProperties.token}",
           new PagerDutyService.PagerDutyCreateEvent(
             service_key: serviceKey,
             client: "Spinnaker (${notification.source.user})",

echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy

@@ -62,7 +62,6 @@ class SlackNotificationService implements NotificationService {
           new SlackAttachment(subject, text, (InteractiveActions)notification.interactiveActions),
           address, true)
       }
-      log.trace("Received response from Slack: {}", response?.string())
     }
 
     new EchoResponse.Void()

echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfig.java

@@ -21,31 +21,27 @@
 import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import retrofit2.Retrofit;
 import retrofit2.converter.jackson.JacksonConverterFactory;
 
 @Configuration
+@EnableConfigurationProperties(PagerDutyConfigurationProperties.class)
 @ConditionalOnProperty("pager-duty.enabled")
 public class PagerDutyConfig {
   private static final Logger log = LoggerFactory.getLogger(PagerDutyConfig.class);
 
-  private final String endpoint;
-
-  public PagerDutyConfig(
-      @Value("${pager-duty.endpoint:https://events.pagerduty.com}") String endpoint) {
-    this.endpoint = endpoint;
-  }
-
   @Bean
-  PagerDutyService pagerDutyService(OkHttp3ClientConfiguration okHttpClientConfig) {
+  PagerDutyService pagerDutyService(
+      OkHttp3ClientConfiguration okHttpClientConfig,
+      PagerDutyConfigurationProperties pageDutyProps) {
     log.info("Pager Duty service loaded");
 
     return new Retrofit.Builder()
-        .baseUrl(endpoint)
+        .baseUrl(pageDutyProps.getEndpoint())
         .client(okHttpClientConfig.createForRetrofit2().build())
         .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
         .addConverterFactory(JacksonConverterFactory.create())

echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfigurationProperties.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2025 OpsMx, 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.echo.config;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+
+@Setter
+@Getter
+@ConfigurationProperties(prefix = "pager-duty")
+public class PagerDutyConfigurationProperties {
+  private String endpoint = "https://events.pagerduty.com";
+  private String token;
+}

echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/CDEventsNotificationAgent.java

@@ -82,10 +82,12 @@ public void sendNotifications(
             response.body() != null ? response.body().string() : "");
       } catch (IOException e) {
         log.info(
-            "Received response from events broker : {} {} for execution id {}",
+            "Received response from events broker : {} {} for execution id {} "
+                + "but unable to serialize the response body: {}",
             response.code(),
             response.message(),
-            executionId);
+            executionId,
+            e.getMessage());
       }
     }
   }

echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/GithubNotificationAgent.java

@@ -24,6 +24,7 @@
 import com.netflix.spinnaker.echo.github.GithubStatus;
 import com.netflix.spinnaker.kork.core.RetrySupport;
 import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall;
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException;
 import java.util.Map;
 import java.util.Optional;
 import java.util.regex.Matcher;
@@ -127,16 +128,20 @@ public void sendNotifications(
   }
 
   private String getBranchCommit(String repo, String sha) {
-    GithubCommit message =
-        Retrofit2SyncCall.execute(githubService.getCommit("token " + token, repo, sha));
-
+    GithubCommit message;
+    try {
+      message = Retrofit2SyncCall.execute(githubService.getCommit("token " + token, repo, sha));
+    } catch (SpinnakerServerException e) {
+      return sha;
+    }
     Pattern pattern =
         Pattern.compile(
             "Merge (?<branchCommit>[0-9a-f]{5,40}) into (?<masterCommit>[0-9a-f]{5,40})");
     Matcher matcher = pattern.matcher(message.getCommit().getMessage());
     if (matcher.matches()) {
       return matcher.group("branchCommit");
     }
+
     return sha;
   }
 

echo-telemetry/src/main/java/com/netflix/spinnaker/echo/config/TelemetryConfig.kt

@@ -15,11 +15,15 @@
  */
 package com.netflix.spinnaker.echo.config
 
+import com.netflix.spinnaker.config.DefaultServiceEndpoint
 import com.netflix.spinnaker.config.OkHttp3ClientConfiguration
+import com.netflix.spinnaker.config.okhttp3.DefaultOkHttpClientBuilderProvider
 import com.netflix.spinnaker.echo.config.TelemetryConfig.TelemetryConfigProps
 import com.netflix.spinnaker.echo.telemetry.TelemetryService
 import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory
+import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties
 import de.huxhorn.sulky.ulid.ULID
+import okhttp3.OkHttpClient
 import org.slf4j.LoggerFactory
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.boot.context.properties.ConfigurationProperties
@@ -41,12 +45,15 @@ open class TelemetryConfig {
   @Bean
   open fun telemetryService(
     configProps: TelemetryConfigProps,
-    okHttpClientConfig: OkHttp3ClientConfiguration
+    okHttpClientConfig: OkHttp3ClientConfiguration,
+    okHttpClient: OkHttpClient
   ): TelemetryService {
+    val clientProps = OkHttpClientConfigurationProperties(configProps.connectionTimeoutMillis.toLong(), configProps.readTimeoutMillis.toLong())
+    val clientProvider = DefaultOkHttpClientBuilderProvider(okHttpClient, clientProps)
     log.info("Telemetry service loaded")
     return Retrofit.Builder()
       .baseUrl(configProps.endpoint)
-      .client(okHttpClientConfig.createForRetrofit2().build())
+      .client(clientProvider.get(DefaultServiceEndpoint("telemetry", configProps.endpoint)).build())
       .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance())
       .addConverterFactory(JacksonConverterFactory.create())
       .build()
@@ -65,6 +72,8 @@ open class TelemetryConfig {
     var instanceId = ULID().nextULID()
     var spinnakerVersion = "unknown"
     var deploymentMethod = DeploymentMethod()
+    var connectionTimeoutMillis = 3000
+    var readTimeoutMillis = 5000
 
     class DeploymentMethod {
       var type: String? = null