This is an automated email from the ASF dual-hosted git repository.
stevel pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new 50e6b49e057 HADOOP-19299. HttpReferrerAuditHeader resilience (#7095)
50e6b49e057 is described below
commit 50e6b49e0574810db26267f2568674a3d441ef52
Author: Steve Loughran <ste...@cloudera.com>
AuthorDate: Mon Oct 7 13:53:01 2024 +0100
HADOOP-19299. HttpReferrerAuditHeader resilience (#7095)
* HttpReferrerAuditHeader is thread safe, copying the lists/maps passed
in and using synchronized methods when necessary.
* All exceptions raised when building referrer header are caught
and swallowed.
* The first such error is logged at warn,
* all errors plus stack are logged at debug
Contributed by Steve Loughran
---
.../fs/store/audit/HttpReferrerAuditHeader.java | 45 +++++++++++++++----
.../fs/s3a/audit/impl/ActiveAuditManagerS3A.java | 12 ++++-
.../hadoop/fs/s3a/audit/impl/LoggingAuditor.java | 16 ++++++-
.../fs/s3a/audit/TestHttpReferrerAuditHeader.java | 33 +++++++++++++-
.../fs/s3a/audit/impl/ReferrerExtractor.java | 52 ++++++++++++++++++++++
5 files changed, 146 insertions(+), 12 deletions(-)
diff --git
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/audit/HttpReferrerAuditHeader.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/audit/HttpReferrerAuditHeader.java
index b2684e75889..01a36b24fb2 100644
---
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/audit/HttpReferrerAuditHeader.java
+++
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/audit/HttpReferrerAuditHeader.java
@@ -29,6 +29,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.StringJoiner;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;
import java.util.stream.Collectors;
@@ -40,6 +41,7 @@ import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.audit.CommonAuditContext;
import org.apache.hadoop.fs.store.LogExactlyOnce;
+import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableSet;
import org.apache.http.NameValuePair;
import org.apache.http.client.utils.URLEncodedUtils;
@@ -57,6 +59,13 @@ import static
org.apache.hadoop.fs.audit.AuditConstants.REFERRER_ORIGIN_HOST;
* {@code org.apache.hadoop.fs.s3a.audit.TestHttpReferrerAuditHeader}
* so as to verify that header generation in the S3A auditors, and
* S3 log parsing, all work.
+ * <p>
+ * This header may be shared across multiple threads at the same time.
+ * so some methods are marked as synchronized, specifically those reading
+ * or writing the attribute map.
+ * <p>
+ * For the same reason, maps and lists passed down during construction are
+ * copied into thread safe structures.
*/
@InterfaceAudience.Private
@InterfaceStability.Unstable
@@ -81,6 +90,14 @@ public final class HttpReferrerAuditHeader {
private static final LogExactlyOnce WARN_OF_URL_CREATION =
new LogExactlyOnce(LOG);
+ /**
+ * Log for warning of an exception raised when building
+ * the referrer header, including building the evaluated
+ * attributes.
+ */
+ private static final LogExactlyOnce ERROR_BUILDING_REFERRER_HEADER =
+ new LogExactlyOnce(LOG);
+
/** Context ID. */
private final String contextId;
@@ -122,7 +139,11 @@ public final class HttpReferrerAuditHeader {
/**
* Instantiate.
- *
+ * <p>
+ * All maps/enums passed down are copied into thread safe equivalents.
+ * as their origin is unknown and cannot be guaranteed to
+ * not be shared.
+ * <p>
* Context and operationId are expected to be well formed
* numeric/hex strings, at least adequate to be
* used as individual path elements in a URL.
@@ -130,15 +151,15 @@ public final class HttpReferrerAuditHeader {
private HttpReferrerAuditHeader(
final Builder builder) {
this.contextId = requireNonNull(builder.contextId);
- this.evaluated = builder.evaluated;
- this.filter = builder.filter;
+ this.evaluated = new ConcurrentHashMap<>(builder.evaluated);
+ this.filter = ImmutableSet.copyOf(builder.filter);
this.operationName = requireNonNull(builder.operationName);
this.path1 = builder.path1;
this.path2 = builder.path2;
this.spanId = requireNonNull(builder.spanId);
// copy the parameters from the builder and extend
- attributes = builder.attributes;
+ attributes = new ConcurrentHashMap<>(builder.attributes);
addAttribute(PARAM_OP, operationName);
addAttribute(PARAM_PATH, path1);
@@ -166,17 +187,18 @@ public final class HttpReferrerAuditHeader {
* per entry, and "" returned.
* @return a referrer string or ""
*/
- public String buildHttpReferrer() {
+ public synchronized String buildHttpReferrer() {
String header;
try {
+ Map<String, String> requestAttrs = new HashMap<>(attributes);
String queries;
// Update any params which are dynamically evaluated
evaluated.forEach((key, eval) ->
- addAttribute(key, eval.get()));
+ requestAttrs.put(key, eval.get()));
// now build the query parameters from all attributes, static and
// evaluated, stripping out any from the filter
- queries = attributes.entrySet().stream()
+ queries = requestAttrs.entrySet().stream()
.filter(e -> !filter.contains(e.getKey()))
.map(e -> e.getKey() + "=" + e.getValue())
.collect(Collectors.joining("&"));
@@ -189,7 +211,14 @@ public final class HttpReferrerAuditHeader {
} catch (URISyntaxException e) {
WARN_OF_URL_CREATION.warn("Failed to build URI for auditor: " + e, e);
header = "";
+ } catch (RuntimeException e) {
+ // do not let failure to build the header stop the request being
+ // issued.
+ ERROR_BUILDING_REFERRER_HEADER.warn("Failed to construct referred header
{}", e.toString());
+ LOG.debug("Full stack", e);
+ header = "";
}
+
return header;
}
@@ -200,7 +229,7 @@ public final class HttpReferrerAuditHeader {
* @param key query key
* @param value query value
*/
- private void addAttribute(String key,
+ private synchronized void addAttribute(String key,
String value) {
if (StringUtils.isNotEmpty(value)) {
attributes.put(key, value);
diff --git
a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/ActiveAuditManagerS3A.java
b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/ActiveAuditManagerS3A.java
index 9dd04af68e8..e8e989efaa1 100644
---
a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/ActiveAuditManagerS3A.java
+++
b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/ActiveAuditManagerS3A.java
@@ -700,7 +700,8 @@ public final class ActiveAuditManagerS3A
* span is deactivated.
* Package-private for testing.
*/
- private final class WrappingAuditSpan extends AbstractAuditSpanImpl {
+ @VisibleForTesting
+ final class WrappingAuditSpan extends AbstractAuditSpanImpl {
/**
* Inner span.
@@ -792,6 +793,15 @@ public final class ActiveAuditManagerS3A
return isValid && span.isValidSpan();
}
+ /**
+ * Get the inner span.
+ * @return the span.
+ */
+ @VisibleForTesting
+ AuditSpanS3A getSpan() {
+ return span;
+ }
+
/**
* Forward to the inner span.
* {@inheritDoc}
diff --git
a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/LoggingAuditor.java
b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/LoggingAuditor.java
index 5f93454795a..16bae4b8164 100644
---
a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/LoggingAuditor.java
+++
b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/audit/impl/LoggingAuditor.java
@@ -38,6 +38,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.audit.AuditConstants;
import org.apache.hadoop.fs.audit.CommonAuditContext;
@@ -252,6 +253,17 @@ public class LoggingAuditor
this.lastHeader = lastHeader;
}
+ /**
+ * Get the referrer provided the span is an instance or
+ * subclass of LoggingAuditSpan.
+ * @param span span
+ * @return the referrer
+ * @throws ClassCastException if a different span type was passed in
+ */
+ @VisibleForTesting
+ HttpReferrerAuditHeader getReferrer(AuditSpanS3A span) {
+ return ((LoggingAuditSpan) span).getReferrer();
+ }
/**
* Span which logs at debug and sets the HTTP referrer on
* invocations.
@@ -441,10 +453,10 @@ public class LoggingAuditor
}
/**
- * Get the referrer; visible for tests.
+ * Get the referrer.
* @return the referrer.
*/
- HttpReferrerAuditHeader getReferrer() {
+ private HttpReferrerAuditHeader getReferrer() {
return referrer;
}
diff --git
a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/TestHttpReferrerAuditHeader.java
b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/TestHttpReferrerAuditHeader.java
index 1dfa94cc5ad..43155fe239f 100644
---
a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/TestHttpReferrerAuditHeader.java
+++
b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/TestHttpReferrerAuditHeader.java
@@ -24,6 +24,7 @@ import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
+import org.assertj.core.api.Assertions;
import software.amazon.awssdk.http.SdkHttpRequest;
import org.junit.Before;
import org.junit.Test;
@@ -32,6 +33,7 @@ import org.slf4j.LoggerFactory;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.s3a.audit.impl.LoggingAuditor;
+import org.apache.hadoop.fs.s3a.audit.impl.ReferrerExtractor;
import org.apache.hadoop.fs.store.audit.AuditSpan;
import org.apache.hadoop.fs.audit.CommonAuditContext;
import org.apache.hadoop.fs.store.audit.HttpReferrerAuditHeader;
@@ -417,4 +419,33 @@ public class TestHttpReferrerAuditHeader extends
AbstractAuditingTest {
.describedAs("Stripped <%s>", str)
.isEqualTo(ex);
}
-}
\ No newline at end of file
+
+ /**
+ * Verify that exceptions raised when building referrer headers
+ * do not result in failures, just an empty header.
+ */
+ @Test
+ public void testSpanResilience() throws Throwable {
+ final CommonAuditContext auditContext =
CommonAuditContext.currentAuditContext();
+ final String failing = "failing";
+ auditContext.put(failing, () -> {
+ throw new RuntimeException("raised");
+ });
+ try {
+ final HttpReferrerAuditHeader referrer =
ReferrerExtractor.getReferrer(auditor, span());
+ Assertions.assertThat(referrer.buildHttpReferrer())
+ .describedAs("referrer header")
+ .isBlank();
+ // repeat
+ LOG.info("second attempt: there should be no second warning below");
+ Assertions.assertThat(referrer.buildHttpReferrer())
+ .describedAs("referrer header 2")
+ .isBlank();
+ referrer.buildHttpReferrer();
+ } finally {
+ // critical to remove this so it doesn't interfere with any other
+ // tests
+ auditContext.remove(failing);
+ }
+ }
+}
diff --git
a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/impl/ReferrerExtractor.java
b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/impl/ReferrerExtractor.java
new file mode 100644
index 00000000000..80d6512c976
--- /dev/null
+++
b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/audit/impl/ReferrerExtractor.java
@@ -0,0 +1,52 @@
+/*
+ * 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.hadoop.fs.s3a.audit.impl;
+
+import org.apache.hadoop.fs.s3a.audit.AuditSpanS3A;
+import org.apache.hadoop.fs.store.audit.HttpReferrerAuditHeader;
+
+/**
+ * Extract the referrer from a LoggingAuditor through a package-private
+ * method.
+ */
+public final class ReferrerExtractor {
+
+ private ReferrerExtractor() {
+ }
+
+ /**
+ * Get the referrer provided the span is an instance or
+ * subclass of LoggingAuditSpan.
+ * If wrapped by a {@code WrappingAuditSpan}, it will be extracted.
+ * @param auditor the auditor.
+ * @param span span
+ * @return the referrer
+ * @throws ClassCastException if a different span type was passed in
+ */
+ public static HttpReferrerAuditHeader getReferrer(LoggingAuditor auditor,
+ AuditSpanS3A span) {
+ AuditSpanS3A sp;
+ if (span instanceof ActiveAuditManagerS3A.WrappingAuditSpan) {
+ sp = ((ActiveAuditManagerS3A.WrappingAuditSpan) span).getSpan();
+ } else {
+ sp = span;
+ }
+ return auditor.getReferrer(sp);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
