Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce log spam: gated logging for high frequency items. #35

Merged
merged 1 commit into from Oct 30, 2023
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
Reduce log spam: gated logging for high frequency items.
  • Loading branch information
jmacxx committed Oct 15, 2023
commit e2cf7e1cdaec6efd19c841e79593e73d0e111735
6 changes: 2 additions & 4 deletions src/main/java/bisq/price/spot/ArsBlueRateTransformer.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@

import bisq.price.spot.providers.BlueRateProvider;
import bisq.price.util.bluelytics.ArsBlueMarketGapProvider;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;

import java.util.Optional;
import java.util.OptionalDouble;

@Component
@Slf4j
public class ArsBlueRateTransformer implements ExchangeRateTransformer {
private final ArsBlueMarketGapProvider blueMarketGapProvider;

Expand Down Expand Up @@ -54,8 +52,8 @@ public Optional<ExchangeRate> apply(ExchangeRateProvider provider, ExchangeRate
originalExchangeRate.getProvider()
);

log.info(String.format("%s transformed from %s to %s",
originalExchangeRate.getCurrency(), originalExchangeRate.getPrice(), blueRate));
provider.getGatedLogging().maybeLogInfo(String.format("%s transformed from %s to %s",
originalExchangeRate.getCurrency(), originalExchangeRate.getPrice(), blueRate));

return Optional.of(newExchangeRate);
}
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/bisq/price/spot/ExchangeRateProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import bisq.core.locale.CurrencyUtil;
import bisq.core.locale.TradeCurrency;
import bisq.price.PriceProvider;
import bisq.price.util.GatedLogging;
import lombok.Getter;
import org.knowm.xchange.Exchange;
import org.knowm.xchange.ExchangeFactory;
import org.knowm.xchange.currency.Currency;
Expand All @@ -32,7 +34,6 @@
import org.knowm.xchange.service.marketdata.params.Params;
import org.springframework.core.env.Environment;

import javax.annotation.Nullable;
import java.io.IOException;
import java.math.BigDecimal;
import java.math.RoundingMode;
Expand All @@ -59,6 +60,8 @@ public abstract class ExchangeRateProvider extends PriceProvider<Set<ExchangeRat
private final String name;
private final String prefix;
private final Environment env;
@Getter
private final GatedLogging gatedLogging = new GatedLogging();

public ExchangeRateProvider(Environment env, String name, String prefix, Duration refreshInterval) {
super(refreshInterval);
Expand Down
35 changes: 20 additions & 15 deletions src/main/java/bisq/price/spot/ExchangeRateService.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

import bisq.common.util.Tuple2;
import bisq.core.util.InlierUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import bisq.price.util.GatedLogging;
import lombok.extern.slf4j.Slf4j;
import org.springframework.core.env.Environment;
import org.springframework.stereotype.Service;

Expand All @@ -32,11 +32,12 @@
* High-level {@link ExchangeRate} data operations.
*/
@Service
@Slf4j
class ExchangeRateService {
protected final Logger log = LoggerFactory.getLogger(this.getClass());
private final Environment env;
private final List<ExchangeRateProvider> providers;
private final List<ExchangeRateTransformer> transformers;
private final GatedLogging gatedLogging = new GatedLogging();

/**
* Construct an {@link ExchangeRateService} with a list of all
Expand Down Expand Up @@ -87,6 +88,7 @@ public Map<String, Object> getAllMarketPrices() {
* by currency code
*/
private Map<String, ExchangeRate> getAggregateExchangeRates() {
boolean maybeLogDetails = gatedLogging.gatingOperation();
Map<String, ExchangeRate> aggregateExchangeRates = new HashMap<>();

// Query all providers and collect all exchange rates, grouped by currency code
Expand All @@ -110,7 +112,7 @@ private Map<String, ExchangeRate> getAggregateExchangeRates() {
} else {
// If multiple providers have rates for this currency, then
// aggregate = average of the rates
double priceAvg = priceAverageWithOutliersRemoved(exchangeRateList, currencyCode);
double priceAvg = priceAverageWithOutliersRemoved(exchangeRateList, currencyCode, maybeLogDetails);
aggregateExchangeRate = new ExchangeRate(
currencyCode,
BigDecimal.valueOf(priceAvg),
Expand All @@ -123,7 +125,8 @@ private Map<String, ExchangeRate> getAggregateExchangeRates() {
return aggregateExchangeRates;
}

private double priceAverageWithOutliersRemoved(List<ExchangeRate> exchangeRateList, String contextInfo) {
private double priceAverageWithOutliersRemoved(
List<ExchangeRate> exchangeRateList, String contextInfo, boolean logOutliers) {
final List<Double> yValues = exchangeRateList.stream().
mapToDouble(ExchangeRate::getPrice).boxed().collect(Collectors.toList());
Tuple2<Double, Double> tuple = InlierUtil.findInlierRange(yValues, 0, getOutlierStdDeviation());
Expand All @@ -145,16 +148,18 @@ private double priceAverageWithOutliersRemoved(List<ExchangeRate> exchangeRateLi
double priceAvg = opt.orElseThrow(IllegalStateException::new);

// log the outlier prices which were removed from the average, if any.
for (ExchangeRate badRate : exchangeRateList.stream()
.filter(e -> !filteredPrices.contains(e))
.collect(Collectors.toList())) {
log.info("{} {} outlier price removed:{}, lower/upper bounds:{}/{}, consensus price:{}",
badRate.getProvider(),
badRate.getCurrency(),
badRate.getPrice(),
lowerBound,
upperBound,
priceAvg);
if (logOutliers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You wouldn't need to add a new method argument and pass the boolean to priceAverageWithOutliersRemoved if you move boolean maybeLogDetails = gatedLogging.gatingOperation(); from line 91 to line 150.

Copy link
Author

Choose a reason for hiding this comment

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

@alvasw : The decision to log has to be taken outside of that loop because we want to log a complete batch of outlier information together. If it were inside the loop the decision would be tested repeatedly, resulting in only the first line logged which does not achieve the purpose.

e.g. desired output when it is time to log:

Oct-25 21:30:22.970 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: KRAKEN DOGE outlier price removed:2.03E-6, lower/upper bounds:2.02E-6/2.021E-6, consensus price:2.0203333333333334E-6 
Oct-25 21:30:22.971 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: KRAKEN DASH outlier price removed:8.0E-4, lower/upper bounds:8.02E-4/8.03E-4, consensus price:8.025E-4 
Oct-25 21:30:22.971 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: IndependentReserve AUD outlier price removed:55266.32, lower/upper bounds:55336.586/55409.0, consensus price:55372.793000000005 
Oct-25 21:30:22.971 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BTCMARKETS AUD outlier price removed:55433.84, lower/upper bounds:55336.586/55409.0, consensus price:55372.793000000005 
Oct-25 21:30:22.971 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BITFINEX TRY outlier price removed:746320.0, lower/upper bounds:977308.448/981000.0, consensus price:979154.2239999999 
Oct-25 21:30:22.971 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BITFINEX EUR outlier price removed:32963.0, lower/upper bounds:32922.0/32925.82, consensus price:32923.64125 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BTCMARKETS LTC outlier price removed:0.002, lower/upper bounds:0.001988/0.001992, consensus price:0.001989608333333333
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: CRYPTOYA: ripioexchange ARS outlier price removed:3.991349663E7, lower/upper bounds:3.132673138E7/3.46499979E7, consensus price:3.223392967352302E7 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: CRYPTOYA: cryptomkt ARS outlier price removed:3.746764154E7, lower/upper bounds:3.132673138E7/3.46499979E7, consensus price:3.223392967352302E7 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: COINGECKO JPY outlier price removed:5221749.391, lower/upper bounds:5214269.0/5219600.0, consensus price:5216934.5 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: KRAKEN JPY outlier price removed:5213293.0, lower/upper bounds:5214269.0/5219600.0, consensus price:5216934.5 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BITFINEX GBP outlier price removed:28764.0, lower/upper bounds:28721.0/28732.442, consensus price:28728.5855 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BITSTAMP USDT-E outlier price removed:2.883E-5, lower/upper bounds:2.882E-5/2.882E-5, consensus price:2.882E-5 
Oct-25 21:30:22.972 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: KRAKEN USDT-E outlier price removed:2.881E-5, lower/upper bounds:2.882E-5/2.882E-5, consensus price:2.882E-5 
Oct-25 21:30:22.973 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BITFINEX USD outlier price removed:34730.0, lower/upper bounds:34713.6/34720.0, consensus price:34716.0936 
Oct-25 21:30:22.973 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: COINBASEPRO ETC outlier price removed:4.77E-4, lower/upper bounds:4.71E-4/4.72E-4, consensus price:4.7138E-4 
Oct-25 21:30:22.973 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BTCMARKETS ETH outlier price removed:0.0518373, lower/upper bounds:0.05161/0.0517, consensus price:0.05165059428571429 
Oct-25 21:30:22.973 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: KRAKEN ZEC outlier price removed:7.9E-4, lower/upper bounds:7.92E-4/7.94E-4, consensus price:7.931133333333333E-4 
Oct-25 21:30:22.973 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: BITFINEX XMR outlier price removed:0.0046327, lower/upper bounds:0.00463/0.00463, consensus price:0.00463 
Oct-25 21:30:22.973 [http-nio-8080-exec-1] INFO  b.p.s.ExchangeRateService: POLO XMR outlier price removed:0.004627, lower/upper bounds:0.00463/0.00463, consensus price:0.00463 

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right! That makes sense.

for (ExchangeRate badRate : exchangeRateList.stream()
.filter(e -> !filteredPrices.contains(e))
.collect(Collectors.toList())) {
log.info("{} {} outlier price removed:{}, lower/upper bounds:{}/{}, consensus price:{}",
badRate.getProvider(),
badRate.getCurrency(),
badRate.getPrice(),
lowerBound,
upperBound,
priceAvg);
}
}
return priceAvg;
}
Expand Down
46 changes: 46 additions & 0 deletions src/main/java/bisq/price/util/GatedLogging.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* This file is part of Bisq.
*
* Bisq is free software: you can redistribute it and/or modify it
* under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or (at
* your option) any later version.
*
* Bisq is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
* License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Bisq. If not, see <http://www.gnu.org/licenses/>.
*/

package bisq.price.util;

import lombok.extern.slf4j.Slf4j;

import java.time.Instant;

/* Per https://github.com/bisq-network/bisq-pricenode/issues/33
* There's too much logging of outlier filtering data, fills up the logs too fast and obliterates other valid logging.
* It correlates with client requests. Change that logging so its output once per minute.
*/

@Slf4j
public class GatedLogging {
private long timestampOfLastLogMessage = 0;

public void maybeLogInfo(String format, Object... params) {
if (gatingOperation()) {
log.info(format, params);
}
}

public boolean gatingOperation() {
if (Instant.now().getEpochSecond() - timestampOfLastLogMessage > 60) {
timestampOfLastLogMessage = Instant.now().getEpochSecond();
return true;
}
return false;
}
}