Skip to content

Commit

Permalink
Added switch to disable MDC context
Browse files Browse the repository at this point in the history
  • Loading branch information
MinnDevelopment committed Dec 16, 2017
1 parent 22bae3f commit 1cf8b40
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,11 @@ public class DefaultShardManager implements ShardManager
*/
protected IntFunction<ConcurrentMap<String, String>> contextProvider;

/**
* Whether to use the MDC context provider.
*/
protected boolean enableMDC;

/**
* Creates a new DefaultShardManager instance.
* @param shardsTotal
Expand Down Expand Up @@ -265,7 +270,7 @@ protected DefaultShardManager(final int shardsTotal, final Collection<Integer> s
final boolean enableShutdownHook, final boolean enableBulkDeleteSplitting,
final boolean autoReconnect, final IntFunction<Boolean> idleProvider,
final boolean retryOnTimeout, final boolean useShutdownNow,
final IntFunction<ConcurrentMap<String, String>> contextProvider)
final boolean enableMDC, final IntFunction<ConcurrentMap<String, String>> contextProvider)
{
this.shardsTotal = shardsTotal;
this.listeners = listeners;
Expand All @@ -288,6 +293,7 @@ protected DefaultShardManager(final int shardsTotal, final Collection<Integer> s
this.retryOnTimeout = retryOnTimeout;
this.useShutdownNow = useShutdownNow;
this.contextProvider = contextProvider;
this.enableMDC = enableMDC;

if (shardsTotal != -1)
{
Expand Down Expand Up @@ -504,8 +510,8 @@ else if (api.getStatus() == JDA.Status.RECONNECT_QUEUED)
protected JDAImpl buildInstance(final int shardId) throws LoginException, RateLimitedException
{
final JDAImpl jda = new JDAImpl(AccountType.BOT, this.token, this.httpClientBuilder, this.wsFactory, this.shardedRateLimiter,
this.autoReconnect, this.enableVoice, false, this.enableBulkDeleteSplitting, retryOnTimeout,
this.corePoolSize, this.maxReconnectDelay, this.contextProvider == null ? null : contextProvider.apply(shardId));
this.autoReconnect, this.enableVoice, false, this.enableBulkDeleteSplitting, this.retryOnTimeout, this.enableMDC,
this.corePoolSize, this.maxReconnectDelay, this.contextProvider == null || !this.enableMDC ? null : contextProvider.apply(shardId));

jda.asBot().setShardManager(this);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class DefaultShardManagerBuilder
{
protected final List<Object> listeners = new ArrayList<>();
protected IntFunction<ConcurrentMap<String, String>> contextProvider = null;
protected boolean enableContext = true;
protected boolean enableBulkDeleteSplitting = true;
protected boolean enableShutdownHook = true;
protected boolean enableVoice = true;
Expand Down Expand Up @@ -83,6 +84,7 @@ public DefaultShardManagerBuilder() {}
* Additionally it will provide context for the id via {@code jda.shard.id} and the total via {@code jda.shard.total}.
*
* <p><b>The manager will call this with a shardId and it is recommended to provide a different context map for each shard!</b>
* <br>This automatically switches {@link #setContextEnabled(boolean)} to true if the provided function is not null!
*
* @param provider
* The provider for <b>modifiable</b> context maps to use in JDA, or {@code null} to reset
Expand All @@ -94,6 +96,27 @@ public DefaultShardManagerBuilder() {}
public DefaultShardManagerBuilder setContextMap(IntFunction<ConcurrentMap<String, String>> provider)
{
this.contextProvider = provider;
if (provider != null)
this.enableContext = true;
return this;
}


/**
* Whether JDA should use a synchronized MDC context for all of its controlled threads.
* <br>Default: {@code true}
*
* @param enable
* True, if JDA should provide an MDC context map
*
* @return The {@link net.dv8tion.jda.bot.sharding.DefaultShardManagerBuilder DefaultShardManagerBuilder} instance. Useful for chaining.
*
* @see <a href="https://www.slf4j.org/api/org/slf4j/MDC.html" target="_blank">MDC Javadoc</a>
* @see #setContextMap(java.util.function.IntFunction)
*/
public DefaultShardManagerBuilder setContextEnabled(boolean enable)
{
this.enableContext = enable;
return this;
}

Expand Down Expand Up @@ -715,7 +738,7 @@ public ShardManager build() throws LoginException, IllegalArgumentException
final DefaultShardManager manager = new DefaultShardManager(this.shardsTotal, this.shards, this.listeners, this.token, this.eventManager,
this.audioSendFactory, this.gameProvider, this.statusProvider, this.httpClientBuilder, this.wsFactory, this.threadFactory, this.shardedRateLimiter,
this.maxReconnectDelay, this.corePoolSize, this.enableVoice, this.enableShutdownHook, this.enableBulkDeleteSplitting,
this.autoReconnect, this.idleProvider, this.retryOnTimeout, this.useShutdownNow, this.contextProvider);
this.autoReconnect, this.idleProvider, this.retryOnTimeout, this.useShutdownNow, this.enableContext, this.contextProvider);

manager.login();

Expand Down
26 changes: 25 additions & 1 deletion src/main/java/net/dv8tion/jda/core/JDABuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class JDABuilder
protected final List<Object> listeners;

protected ConcurrentMap<String, String> contextMap = null;
protected boolean enableContext = true;
protected SessionReconnectQueue reconnectQueue = null;
protected ShardedRateLimiter shardRateLimiter = null;
protected OkHttpClient.Builder httpClientBuilder = null;
Expand Down Expand Up @@ -95,16 +96,39 @@ public JDABuilder(AccountType accountType)
* where {@code SHARD_ID} and {@code TOTAL} are the shard configuration.
* Additionally it will provide context for the id via {@code jda.shard.id} and the total via {@code jda.shard.total}.
*
* <p>If provided with non-null map this automatically enables MDC context using {@link #setContextEnabled(boolean) setContextEnable(true)}!
*
* @param map
* The <b>modifiable</b> context map to use in JDA, or {@code null} to reset
*
* @return The JDABuilder instance. Useful for chaining.
*
* @see <a href="https://www.slf4j.org/api/org/slf4j/MDC.html" target="_blank">MDC Javadoc</a>
* @see #setContextEnabled(boolean)
*/
public JDABuilder setContextMap(ConcurrentMap<String, String> map)
{
this.contextMap = map;
if (map != null)
this.enableContext = true;
return this;
}

/**
* Whether JDA should use a synchronized MDC context for all of its controlled threads.
* <br>Default: {@code true}
*
* @param enable
* True, if JDA should provide an MDC context map
*
* @return The JDABuilder instance. Useful for chaining.
*
* @see <a href="https://www.slf4j.org/api/org/slf4j/MDC.html" target="_blank">MDC Javadoc</a>
* @see #setContextMap(java.util.concurrent.ConcurrentMap)
*/
public JDABuilder setContextEnabled(boolean enable)
{
this.enableContext = enable;
return this;
}

Expand Down Expand Up @@ -573,7 +597,7 @@ public JDA buildAsync() throws LoginException, IllegalArgumentException, RateLim
OkHttpClient.Builder httpClientBuilder = this.httpClientBuilder == null ? new OkHttpClient.Builder() : this.httpClientBuilder;
WebSocketFactory wsFactory = this.wsFactory == null ? new WebSocketFactory() : this.wsFactory;
JDAImpl jda = new JDAImpl(accountType, token, httpClientBuilder, wsFactory, shardRateLimiter, autoReconnect, enableVoice, enableShutdownHook,
enableBulkDeleteSplitting, requestTimeoutRetry, corePoolSize, maxReconnectDelay, contextMap);
enableBulkDeleteSplitting, requestTimeoutRetry, enableContext, corePoolSize, maxReconnectDelay, contextMap);

if (eventManager != null)
jda.setEventManager(eventManager);
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/net/dv8tion/jda/core/audio/AudioConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ public void ready()
{
Thread readyThread = new Thread(AudioManagerImpl.AUDIO_THREADS, () ->
{
MDC.setContextMap(contextMap);
if (contextMap != null)
MDC.setContextMap(contextMap);
final long timeout = getGuild().getAudioManager().getConnectTimeout();

final long started = System.currentTimeMillis();
Expand Down Expand Up @@ -315,7 +316,8 @@ private synchronized void setupReceiveThread()
{
receiveThread = new Thread(AudioManagerImpl.AUDIO_THREADS, () ->
{
MDC.setContextMap(contextMap);
if (contextMap != null)
MDC.setContextMap(contextMap);
try
{
udpSocket.setSoTimeout(1000);
Expand Down Expand Up @@ -448,7 +450,8 @@ private synchronized void setupCombinedExecutor()
{
Runnable r = () ->
{
MDC.setContextMap(contextMap);
if (contextMap != null)
MDC.setContextMap(contextMap);
task.run();
};
final Thread t = new Thread(AudioManagerImpl.AUDIO_THREADS, r, threadIdentifier + " Combined Thread");
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/net/dv8tion/jda/core/audio/AudioWebSocket.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ protected void send(int op, Object data)
public void onConnected(WebSocket websocket, Map<String, List<String>> headers)
{
//writing thread
MDC.setContextMap(api.getContextMap());
if (api.getContextMap() != null)
MDC.setContextMap(api.getContextMap());
if (shutdown)
{
//Somehow this AudioWebSocket was shutdown before we finished connecting....
Expand All @@ -134,7 +135,8 @@ public void onConnected(WebSocket websocket, Map<String, List<String>> headers)
public void onTextMessage(WebSocket websocket, String message)
{
//reading thread
MDC.setContextMap(api.getContextMap());
if (api.getContextMap() != null)
MDC.setContextMap(api.getContextMap());
JSONObject contentAll = new JSONObject(message);
int opCode = contentAll.getInt("op");

Expand Down Expand Up @@ -647,7 +649,8 @@ public Thread newThread(Runnable r)
{
Runnable r2 = () ->
{
MDC.setContextMap(contextMap);
if (contextMap != null)
MDC.setContextMap(contextMap);
r.run();
};
final Thread t = new Thread(AudioManagerImpl.AUDIO_THREADS, r2, identifier + " - Thread " + threadCount.getAndIncrement());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import net.dv8tion.jda.core.utils.JDALogger;
import org.slf4j.MDC;

import javax.annotation.CheckForNull;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.NoRouteToHostException;
Expand All @@ -45,7 +46,7 @@ public DefaultSendSystem(IPacketProvider packetProvider)
}

@Override
public void setContextMap(ConcurrentMap<String, String> contextMap)
public void setContextMap(@CheckForNull ConcurrentMap<String, String> contextMap)
{
this.contextMap = contextMap;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package net.dv8tion.jda.core.audio.factory;

import javax.annotation.CheckForNull;
import java.util.concurrent.ConcurrentMap;

/**
Expand Down Expand Up @@ -52,7 +53,7 @@ public interface IAudioSendSystem
* <br>This is guaranteed to be called before {@link #start()}.
*
* @param contextMap
* The JDA internal MDC context map
* The JDA internal MDC context map, or {@code null} if disabled
*/
default void setContextMap(ConcurrentMap<String, String> contextMap) {}
default void setContextMap(@CheckForNull ConcurrentMap<String, String> contextMap) {}
}
28 changes: 18 additions & 10 deletions src/main/java/net/dv8tion/jda/core/entities/impl/JDAImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public class JDAImpl implements JDA

public JDAImpl(AccountType accountType, String token, OkHttpClient.Builder httpClientBuilder, WebSocketFactory wsFactory, ShardedRateLimiter rateLimiter,
boolean autoReconnect, boolean audioEnabled, boolean useShutdownHook, boolean bulkDeleteSplittingEnabled,
boolean retryOnTimeout, int corePoolSize, int maxReconnectDelay, ConcurrentMap<String, String> contextMap)
boolean retryOnTimeout, boolean enableMDC, int corePoolSize, int maxReconnectDelay, ConcurrentMap<String, String> contextMap)
{
this.accountType = accountType;
this.setToken(token);
Expand All @@ -118,7 +118,10 @@ public JDAImpl(AccountType accountType, String token, OkHttpClient.Builder httpC
this.bulkDeleteSplittingEnabled = bulkDeleteSplittingEnabled;
this.pool = new ScheduledThreadPoolExecutor(corePoolSize, new JDAThreadFactory());
this.maxReconnectDelay = maxReconnectDelay;
this.contextMap = contextMap == null ? new ConcurrentHashMap<>() : contextMap;
if (enableMDC)
this.contextMap = contextMap == null ? new ConcurrentHashMap<>() : contextMap;
else
this.contextMap = null;

this.presence = new PresenceImpl(this);
this.requester = new Requester(this, rateLimiter);
Expand All @@ -137,15 +140,19 @@ public int login(String gatewayUrl, ShardInfo shardInfo, SessionReconnectQueue r
if (token == null || token.isEmpty())
throw new LoginException("Provided token was null or empty!");

if (shardInfo != null)
Map<String, String> previousContext = null;
if (contextMap != null)
{
contextMap.put("jda.shard", shardInfo.getShardString());
contextMap.put("jda.shard.id", String.valueOf(shardInfo.getShardId()));
contextMap.put("jda.shard.total", String.valueOf(shardInfo.getShardTotal()));
if (shardInfo != null)
{
contextMap.put("jda.shard", shardInfo.getShardString());
contextMap.put("jda.shard.id", String.valueOf(shardInfo.getShardId()));
contextMap.put("jda.shard.total", String.valueOf(shardInfo.getShardTotal()));
}
// set MDC metadata for build thread
previousContext = MDC.getCopyOfContextMap();
contextMap.forEach(MDC::put);
}
// set MDC metadata for build thread
Map<String, String> previousContext = MDC.getCopyOfContextMap();
contextMap.forEach(MDC::put);
verifyToken();
LOG.info("Login Successful!");

Expand Down Expand Up @@ -786,7 +793,8 @@ public Thread newThread(Runnable r)
{
final Thread thread = new Thread(() ->
{
MDC.setContextMap(contextMap);
if (contextMap != null)
MDC.setContextMap(contextMap);
r.run();
}, "JDA-Thread " + getIdentifierString());
thread.setDaemon(true);
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/net/dv8tion/jda/core/requests/RateLimiter.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void forceShutdown()
private class RateLimitThreadFactory implements ThreadFactory
{
final String identifier;
AtomicInteger threadCount = new AtomicInteger(1);
final AtomicInteger threadCount = new AtomicInteger(1);

public RateLimitThreadFactory(JDAImpl api)
{
Expand All @@ -100,7 +100,8 @@ public Thread newThread(Runnable r)
{
Thread t = new Thread(() ->
{
MDC.setContextMap(requester.api.getContextMap());
if (requester.api.getContextMap() != null)
MDC.setContextMap(requester.api.getContextMap());
r.run();
}, identifier + " - Thread " + threadCount.getAndIncrement());
t.setDaemon(true);
Expand Down
12 changes: 8 additions & 4 deletions src/main/java/net/dv8tion/jda/core/requests/WebSocketClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ private void setupSendingThread()
{
ratelimitThread = new Thread(() ->
{
MDC.setContextMap(api.getContextMap());
if (api.getContextMap() != null)
MDC.setContextMap(api.getContextMap());
boolean needRatelimit;
boolean attemptedToSend;
boolean queueLocked = false;
Expand Down Expand Up @@ -457,7 +458,8 @@ protected void connect()
public void onConnected(WebSocket websocket, Map<String, List<String>> headers)
{
//writing thread
MDC.setContextMap(api.getContextMap());
if (api.getContextMap() != null)
MDC.setContextMap(api.getContextMap());
api.setStatus(JDA.Status.IDENTIFYING_SESSION);
LOG.info("Connected to WebSocket");
if (headers.containsKey("cf-ray"))
Expand Down Expand Up @@ -644,7 +646,8 @@ public void reconnect(boolean callFromQueue, boolean shouldHandleIdentify)
public void onTextMessage(WebSocket websocket, String message)
{
//reading thread
MDC.setContextMap(api.getContextMap());
if (api.getContextMap() != null)
MDC.setContextMap(api.getContextMap());
JSONObject content = new JSONObject(message);
try
{
Expand Down Expand Up @@ -708,7 +711,8 @@ protected void setupKeepAlive(long timeout)
{
keepAliveThread = new Thread(() ->
{
MDC.setContextMap(api.getContextMap());
if (api.getContextMap() != null)
MDC.setContextMap(api.getContextMap());
while (connected)
{
try
Expand Down

0 comments on commit 1cf8b40

Please sign in to comment.