spuru9 commented on code in PR #1187: URL: https://github.com/apache/flink-kubernetes-operator/pull/1187#discussion_r3815771771
########## flink-autoscaler/src/test/java/org/apache/flink/autoscaler/utils/PartitionMetricNameParserTest.java: ########## @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.flink.autoscaler.utils; + +import org.junit.jupiter.api.Test; + +import java.time.Duration; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; + +/** Test for {@link PartitionMetricNameParser}. */ +public class PartitionMetricNameParserTest { Review Comment: Done. Added `testEquivalentToLegacyRegex`: the original regex + its key-extraction logic are now embedded in the test (`LEGACY_REGEX` / `legacyPartitionKey`), and the test asserts the old regex and the new parser return byte-for-byte identical results across the captured real-world fixtures plus ~2500 deterministically-generated realistic names (Kafka with/without cluster, Pulsar, and near-miss negatives). The adversarial payload stays in its own test since the whole point is that the old regex hangs on it while the parser returns quickly. ########## flink-autoscaler/src/main/java/org/apache/flink/autoscaler/utils/PartitionMetricNameParser.java: ########## @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.flink.autoscaler.utils; + +/** + * Parses Kafka/Pulsar source-partition metric names to extract a stable per-partition key. + * + * <p>Uses plain string splitting/indexing rather than a regex, since metric names are fully + * job-controlled and this must stay linear-time regardless of input shape. + */ +public final class PartitionMetricNameParser { Review Comment: There are no e2e tests exercising this path against a real Kafka/Pulsar instance today — the autoscaler e2e (`e2e-tests/test_autoscaler.sh`) runs StateMachineExample, which has no external source, so these partition metrics never appear there. The existing coverage for these metric-name shapes is the set of captured real-world strings in `MetricsCollectionAndEvaluationTest#testKafkaPulsarNumPartitions`. To directly address the "breaking something" concern I leaned on the differential test above (`testEquivalentToLegacyRegex`): it proves the new parser is byte-for-byte equivalent to the old regex across the captured real names plus a large generated set, so behavior is preserved for anything the two agreed on. Happy to add a real Kafka/Pulsar e2e if you'd prefer, though it's a heavier lift — let me know. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
