You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wa...@apache.org on 2026/08/05 08:18:24 UTC
(skywalking) branch pagerDuty created (now dc62d43698)
This is an automated email from the ASF dual-hosted git repository.
wankai123 pushed a change to branch pagerDuty
in repository https://gitbox.apache.org/repos/asf/skywalking.git
at dc62d43698 Address review: validate the PagerDuty events-api-url at config load
This branch includes the following new commits:
new dc62d43698 Address review: validate the PagerDuty events-api-url at config load
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(skywalking) 01/01: Address review: validate the PagerDuty events-api-url at config load
Posted by wa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
wankai123 pushed a commit to branch pagerDuty
in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit dc62d436988432e2320f386f607f2952ea0bbe4f
Author: wankai123 <wa...@foxmail.com>
AuthorDate: Wed Aug 5 16:18:09 2026 +0800
Address review: validate the PagerDuty events-api-url at config load
Reject a malformed or non-http(s) events-api-url while alarm-settings.yml is
read, so a typo fails startup (or the dynamic config update) with a message
naming the setting. Left unvalidated it reached URI.create once per alarm inside
PagerDutyHookCallback, where the surrounding catch dropped every alarm with only
an error log. A scheme-less value such as events.pagerduty.com/v2/enqueue parses
as a valid relative URI, so the check also requires an absolute URL with a host.
The non-https warning moves into the same method, where it now applies only to an
explicitly configured endpoint - the default is always https.
Also reword the changelog entry, which read 'posting to ... defaults to ...'.
---
docs/en/changes/changes.md | 2 +-
.../server/core/alarm/provider/RulesReader.java | 47 +++++++++++++++++-----
.../core/alarm/provider/RulesReaderTest.java | 45 +++++++++++++++++++++
3 files changed, 84 insertions(+), 10 deletions(-)
diff --git a/docs/en/changes/changes.md b/docs/en/changes/changes.md
index d13b3d65d5..5714d37c68 100644
--- a/docs/en/changes/changes.md
+++ b/docs/en/changes/changes.md
@@ -341,7 +341,7 @@
* Support BanyanDB's group-scoped trace retention pipeline in `bydb.yml`. The `trace` and `zipkinTrace` groups gain a `pipeline` block (`enabled`, `enabledEvents`, `mergeGraceSeconds`, `finalizeGraceSeconds`, and an ordered `plugins` chain) that OAP pushes onto the BanyanDB group as a `TracePipelineConfig`, letting a sampler plugin drop traces inside the data node during Hot-phase compaction — after storage, so it reclaims space already written and decides per whole trace, unlike the ing [...]
* Fix: a blank value in `bydb.yml` (`key:` with nothing after it) aborted OAP startup with an opaque `NullPointerException` from `java.util.Properties`, which rejects null values. The BanyanDB config loader now skips blank entries and leaves the field at its default, the same outcome as omitting the line.
* Route LAL rules within a layer by their input type, so a single layer can host rules over different proto inputs. Each compiled rule now carries its effective input type (the proto class its `parsed.*` getters cast to, or `null` for parser-based / untyped rules), and `LogFilterListener` skips any rule whose type doesn't match the incoming log instead of running every rule in the layer. This fixes a latent `ClassCastException` (caught and logged per log) that fired whenever a `MESH` log [...]
-* Fix the PagerDuty alarm hook posting to the Events API v2 endpoint defaults to `https://events.pagerduty.com/v2/enqueue`.
+* Fix the PagerDuty alarm hook to default its Events API v2 endpoint to `https://events.pagerduty.com/v2/enqueue`.
* Fix `HttpAlarmCallback` logging a successful alarm delivery as a failure. The shared HTTP hook helper treated only `200` and `204` as success, so any other 2xx — notably the `202 Accepted` returned by asynchronous intake APIs such as PagerDuty's Events API v2 — produced `send to ... failure. Response code: 202` at ERROR level on every delivered alarm. The alarm was still delivered; the log entry was wrong. The check now accepts the whole 2xx range, for all alarm hooks.
* Make the PagerDuty Events API v2 endpoint configurable through a new optional `events-api-url` setting on each `pagerduty` hook, defaulting to the US service region endpoint. An account in PagerDuty's EU service region can now point the hook straight at `https://events.eu.pagerduty.com/v2/enqueue` rather than relying on PagerDuty forwarding the request — and the routing key and alarm payload from an EU-region account no longer transit the US region.
diff --git a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java
index 7faf47c2d1..d9e92f2cdc 100644
--- a/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java
+++ b/oap-server/server-alarm-plugin/src/main/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReader.java
@@ -20,6 +20,8 @@ package org.apache.skywalking.oap.server.core.alarm.provider;
import java.io.InputStream;
import java.io.Reader;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -383,6 +385,39 @@ public class RulesReader {
});
}
+ /**
+ * Rejects a malformed `events-api-url` while the config is being read, so a typo fails startup (or the dynamic
+ * config update) instead of surfacing later as a per-alarm {@link URI#create} failure inside the hook callback,
+ * where every alarm would be dropped with only an error log.
+ *
+ * @param url the configured endpoint
+ * @param hookName the hook the endpoint belongs to, for the error message
+ */
+ private void validateEventsApiUrl(String url, String hookName) {
+ final URI uri;
+ try {
+ uri = new URI(url);
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(
+ "PagerDuty hook: [" + hookName + "] events-api-url is malformed: [" + url + "].", e);
+ }
+ final String scheme = uri.getScheme();
+ if (uri.getHost() == null
+ || !("https".equalsIgnoreCase(scheme) || "http".equalsIgnoreCase(scheme))) {
+ throw new IllegalArgumentException(
+ "PagerDuty hook: [" + hookName + "] events-api-url must be an absolute http(s) URL, but was: ["
+ + url + "].");
+ }
+ // The integration key travels in the request body, so a non-TLS endpoint puts a credential on the wire.
+ if (!"https".equalsIgnoreCase(scheme)) {
+ log.warn(
+ "PagerDuty hook [{}] is configured with a non-https events-api-url [{}]. "
+ + "The integration key is sent in the request body and will not be encrypted.",
+ hookName, url
+ );
+ }
+ }
+
/**
* Read PagerDuty hook config into {@link PagerDutySettings}
*/
@@ -403,15 +438,9 @@ public class RulesReader {
Object eventsApiUrl = config.get("events-api-url");
if (eventsApiUrl != null && StringUtil.isNotBlank(eventsApiUrl.toString())) {
- settings.setEventsApiUrl(eventsApiUrl.toString().trim());
- }
- // The integration key travels in the request body, so a non-TLS endpoint puts a credential on the wire.
- if (!settings.getEventsApiUrl().startsWith("https://")) {
- log.warn(
- "PagerDuty hook [{}] is configured with a non-https events-api-url [{}]. "
- + "The integration key is sent in the request body and will not be encrypted.",
- settings.getFormattedName(), settings.getEventsApiUrl()
- );
+ final String url = eventsApiUrl.toString().trim();
+ validateEventsApiUrl(url, settings.getFormattedName());
+ settings.setEventsApiUrl(url);
}
List<String> integrationKeys = (List<String>) config.get("integration-keys");
diff --git a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReaderTest.java b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReaderTest.java
index 02da09d0c9..bf517f4f9f 100644
--- a/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReaderTest.java
+++ b/oap-server/server-alarm-plugin/src/test/java/org/apache/skywalking/oap/server/core/alarm/provider/RulesReaderTest.java
@@ -32,11 +32,14 @@ import org.apache.skywalking.oap.server.core.storage.annotation.ValueColumnMetad
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import java.io.StringReader;
import java.util.List;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
public class RulesReaderTest {
@BeforeEach
@@ -47,6 +50,48 @@ public class RulesReaderTest {
"endpoint_percent", "testColumn", Column.ValueDataType.COMMON_VALUE, 0, Scope.Endpoint.getScopeId());
}
+ /**
+ * A bad endpoint must fail while the config is read. Left to the hook callback it would reach
+ * {@code URI.create} once per alarm, where every alarm is dropped with only an error log.
+ */
+ @Test
+ public void testReadPagerDutyRejectsInvalidEventsApiUrl() {
+ for (String invalid : new String[] {
+ "not a valid url",
+ "events.pagerduty.com/v2/enqueue",
+ "ftp://events.pagerduty.com/v2/enqueue"
+ }) {
+ IllegalArgumentException e = assertThrows(
+ IllegalArgumentException.class,
+ () -> new RulesReader(new StringReader(pagerDutyConfig(invalid)), null).readRules(),
+ "an events-api-url of [" + invalid + "] must be rejected at config load"
+ );
+ assertTrue(
+ e.getMessage().contains("events-api-url"),
+ "the error must name the offending setting, but was: " + e.getMessage());
+ }
+ }
+
+ @Test
+ public void testReadPagerDutyAcceptsValidEventsApiUrl() {
+ Rules rules = new RulesReader(
+ new StringReader(pagerDutyConfig("https://events.eu.pagerduty.com/v2/enqueue")), null).readRules();
+ assertEquals(
+ "https://events.eu.pagerduty.com/v2/enqueue",
+ rules.getPagerDutySettingsMap().get(AlarmHooksType.pagerduty.name() + ".default").getEventsApiUrl());
+ }
+
+ private static String pagerDutyConfig(String eventsApiUrl) {
+ return "hooks:\n"
+ + " pagerduty:\n"
+ + " default:\n"
+ + " is-default: true\n"
+ + " text-template: \"Apache SkyWalking Alarm: %s\"\n"
+ + " events-api-url: \"" + eventsApiUrl + "\"\n"
+ + " integration-keys:\n"
+ + " - dummy_key\n";
+ }
+
@Test
public void testReadRules() {
RulesReader reader = new RulesReader(this.getClass()