Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

import com.google.common.eventbus.Subscribe;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.WatchedUpdateResult;

/**
Expand Down Expand Up @@ -232,15 +233,21 @@ private static void addMappingToSpring(Environment environment) {

private void addDynamicConfigurationToSpring(Environment environment,
ConfigCenterConfigurationSource configCenterConfigurationSource) {
if (environment instanceof ConfigurableEnvironment) {
ConfigurableEnvironment ce = (ConfigurableEnvironment) environment;
if (configCenterConfigurationSource != null) {
try {
ce.getPropertySources()
.addFirst(new MapPropertySource("dynamic-source", dynamicData));
} catch (Exception e) {
LOGGER.warn("set up spring property source failed. msg: {}", e.getMessage());
}
if (!(environment instanceof ConfigurableEnvironment)) {
return;
}
ConfigurableEnvironment ce = (ConfigurableEnvironment) environment;
if (configCenterConfigurationSource == null) {
return;
}
try {
ce.getPropertySources().addFirst(new MapPropertySource("dynamic-source", dynamicData));
} catch (Exception e) {
if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
false).get()) {
LOGGER.warn("set up spring property source failed.", e);
} else {
LOGGER.warn("set up spring property source failed. msg: {}", e.getMessage());
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/servicecomb/core/Const.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,7 @@ private Const() {
public static final String AUTH_TOKEN = "x-cse-auth-rsatoken";

public static final String TRACE_ID_NAME = "X-B3-TraceId";

// controlling whether to print stack information with sensitive errors
public static final String PRINT_SENSITIVE_ERROR_MESSAGE = "servicecomb.error.printSensitiveErrorMessage";
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.lang.reflect.InvocationTargetException;
import java.util.concurrent.CompletableFuture;

import org.apache.servicecomb.core.Const;
import org.apache.servicecomb.core.Handler;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.core.exception.ExceptionUtils;
Expand All @@ -34,6 +35,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.netflix.config.DynamicPropertyFactory;

public class ProducerOperationHandler implements Handler {
private static final Logger LOGGER = LoggerFactory.getLogger(ProducerOperationHandler.class);

Expand Down Expand Up @@ -125,10 +128,15 @@ public Response doInvoke(Invocation invocation, SwaggerProducerOperation produce
invocation.onBusinessMethodFinish();
invocation.onBusinessFinish();
} catch (Throwable e) {
if (shouldPrintErrorLog(e)) {
invocation.getTraceIdLogger().error(LOGGER, "unexpected error operation={}, message={}",
invocation.getInvocationQualifiedName(),
org.apache.servicecomb.foundation.common.utils.ExceptionUtils.getExceptionMessageWithoutTrace(e));
if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
Copy link
Copy Markdown

@tjlizz tjlizz Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请问为什么servicecomb.error.printSensitiveErrorMessage=true的时候不使用用invocation.getTraceIdLogger().error打印日志呢?这样打出的异常堆栈没有trace id,根据trace id 查询错误日志的时候会漏掉这个信息呢

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lizeze I think it's a bug, and I create #3220 to fix this

false).get()) {
LOGGER.error("unexpected error operation={}", invocation.getInvocationQualifiedName(), e);
} else {
if (shouldPrintErrorLog(e)) {
invocation.getTraceIdLogger().error(LOGGER, "unexpected error operation={}, message={}",
invocation.getInvocationQualifiedName(),
org.apache.servicecomb.foundation.common.utils.ExceptionUtils.getExceptionMessageWithoutTrace(e));
}
}
invocation.onBusinessMethodFinish();
invocation.onBusinessFinish();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

package org.apache.servicecomb.bizkeeper;

import org.apache.servicecomb.core.Const;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.swagger.invocation.Response;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.netflix.config.DynamicPropertyFactory;
import com.netflix.hystrix.HystrixObservableCommand;
import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext;

Expand Down Expand Up @@ -65,7 +67,12 @@ protected Observable<Response> resumeWithFallback() {
f.onNext(FallbackPolicyManager.getFallbackResponse(type, cause, invocation));
f.onCompleted();
} catch (Exception e) {
LOG.warn("fallback failed due to:" + e.getMessage());
if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
false).get()) {
LOG.warn("fallback failed.", e);
} else {
LOG.warn("fallback failed due to: {}", e.getMessage());
}
throw e;
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Map.Entry;

import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.core.Const;
import org.apache.servicecomb.registry.api.registry.Microservice;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -70,14 +71,14 @@ public boolean isAllowed(Microservice microservice) {
}

private boolean whiteAllowed(Microservice microservice) {
if(whiteList.isEmpty()) {
if (whiteList.isEmpty()) {
return true;
}
return matchFound(microservice, whiteList);
}

private boolean blackDenied(Microservice microservice) {
if(blackList.isEmpty()) {
if (blackList.isEmpty()) {
return false;
}
return matchFound(microservice, blackList);
Expand All @@ -88,8 +89,9 @@ private boolean matchFound(Microservice microservice, Map<String, ConfigurationI
for (ConfigurationItem item : ruleList.values()) {
if (ConfigurationItem.CATEGORY_PROPERTY.equals(item.category)) {
// we support to configure properties, e.g. serviceName, appId, environment, alias, version and so on, also support key in properties.
if (matchMicroserviceField(microservice, item) || matchMicroserviceProperties(microservice, item))
if (matchMicroserviceField(microservice, item) || matchMicroserviceProperties(microservice, item)) {
return true;
}
}
}
return matched;
Expand All @@ -98,8 +100,9 @@ private boolean matchFound(Microservice microservice, Map<String, ConfigurationI
private boolean matchMicroserviceProperties(Microservice microservice, ConfigurationItem item) {
Map<String, String> properties = microservice.getProperties();
for (Entry<String, String> entry : properties.entrySet()) {
if (!entry.getKey().equals(item.propertyName))
if (!entry.getKey().equals(item.propertyName)) {
continue;
}
return isPatternMatch(entry.getValue(), item.rule);
}
return false;
Expand All @@ -110,11 +113,19 @@ private boolean matchMicroserviceField(Microservice microservice, ConfigurationI
try {
fieldValue = new PropertyDescriptor(item.propertyName, Microservice.class).getReadMethod().invoke(microservice);
} catch (Exception e) {
LOG.warn("can't find propertyname: {} in microservice field, will search in microservice properties.", item.propertyName);
if (DynamicPropertyFactory.getInstance().getBooleanProperty(Const.PRINT_SENSITIVE_ERROR_MESSAGE,
false).get()) {
LOG.warn("can't find propertyname: {} in microservice field, will search in microservice properties.",
item.propertyName, e);
} else {
LOG.warn("can't find propertyname: {} in microservice field, will search in microservice properties.",
item.propertyName);
}
return false;
}
if (fieldValue.getClass().getName().equals(String.class.getName()))
if (fieldValue.getClass().getName().equals(String.class.getName())) {
return isPatternMatch((String) fieldValue, item.rule);
}
return false;
}

Expand Down Expand Up @@ -182,9 +193,9 @@ private void logConfigurations(Map<String, ConfigurationItem> configurations, bo
configurations.entrySet().forEach(stringConfigurationItemEntry -> {
ConfigurationItem item = stringConfigurationItemEntry.getValue();
LOG.info((isWhite ? "White list " : "Black list ") + "config item: key=" + stringConfigurationItemEntry.getKey()
+ ";category=" + item.category
+ ";propertyName=" + item.propertyName
+ ";rule=" + item.rule);
+ ";category=" + item.category
+ ";propertyName=" + item.propertyName
+ ";rule=" + item.rule);
});
}
}