-
Notifications
You must be signed in to change notification settings - Fork 336
Have a fully JMS 1.0 compatible instrumentation #11413
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
Open
amarziali
wants to merge
2
commits into
master
Choose a base branch
from
andrea.marziali/jms
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+850
−15
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
130 changes: 130 additions & 0 deletions
130
...nt/instrumentation/jms/javax-jms-1.1/src/testFixtures/java/jms10mock/Jms10Connection.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| package jms10mock; | ||
|
|
||
| import javax.jms.Connection; | ||
| import javax.jms.ConnectionConsumer; | ||
| import javax.jms.ConnectionMetaData; | ||
| import javax.jms.Destination; | ||
| import javax.jms.ExceptionListener; | ||
| import javax.jms.JMSException; | ||
| import javax.jms.Queue; | ||
| import javax.jms.QueueConnection; | ||
| import javax.jms.QueueSession; | ||
| import javax.jms.ServerSessionPool; | ||
| import javax.jms.Session; | ||
| import javax.jms.Topic; | ||
| import javax.jms.TopicConnection; | ||
| import javax.jms.TopicSession; | ||
|
|
||
| /** Wraps a real {@link Connection} but simulates a JMS 1.0 provider. */ | ||
| public class Jms10Connection implements QueueConnection, TopicConnection { | ||
| private final Connection delegate; | ||
|
|
||
| public Jms10Connection(Connection delegate) { | ||
| this.delegate = delegate; | ||
| } | ||
|
|
||
| // --- JMS 1.1-only unified Connection method --- | ||
|
|
||
| @Override | ||
| public Session createSession(boolean transacted, int acknowledgeMode) throws JMSException { | ||
| throw new AbstractMethodError( | ||
| "JMS 1.0 provider does not implement createSession(boolean, int) on Connection"); | ||
| } | ||
|
|
||
| // --- JMS 1.0 QueueConnection methods --- | ||
|
|
||
| @Override | ||
| public QueueSession createQueueSession(boolean transacted, int acknowledgeMode) | ||
| throws JMSException { | ||
| return new Jms10Session(delegate.createSession(transacted, acknowledgeMode)); | ||
| } | ||
|
|
||
| // --- JMS 1.0 TopicConnection methods --- | ||
|
|
||
| @Override | ||
| public TopicSession createTopicSession(boolean transacted, int acknowledgeMode) | ||
| throws JMSException { | ||
| return new Jms10Session(delegate.createSession(transacted, acknowledgeMode)); | ||
| } | ||
|
|
||
| // --- Common Connection methods --- | ||
|
|
||
| @Override | ||
| public String getClientID() throws JMSException { | ||
| return delegate.getClientID(); | ||
| } | ||
|
|
||
| @Override | ||
| public void setClientID(String clientID) throws JMSException { | ||
| delegate.setClientID(clientID); | ||
| } | ||
|
|
||
| @Override | ||
| public ConnectionMetaData getMetaData() throws JMSException { | ||
| return delegate.getMetaData(); | ||
| } | ||
|
|
||
| @Override | ||
| public ExceptionListener getExceptionListener() throws JMSException { | ||
| return delegate.getExceptionListener(); | ||
| } | ||
|
|
||
| @Override | ||
| public void setExceptionListener(ExceptionListener listener) throws JMSException { | ||
| delegate.setExceptionListener(listener); | ||
| } | ||
|
|
||
| @Override | ||
| public void start() throws JMSException { | ||
| delegate.start(); | ||
| } | ||
|
|
||
| @Override | ||
| public void stop() throws JMSException { | ||
| delegate.stop(); | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws JMSException { | ||
| delegate.close(); | ||
| } | ||
|
|
||
| // --- ConnectionConsumer methods — not commonly used, throw for JMS 1.1 unified form --- | ||
|
|
||
| @Override | ||
| public ConnectionConsumer createConnectionConsumer( | ||
| Destination destination, | ||
| String messageSelector, | ||
| ServerSessionPool sessionPool, | ||
| int maxMessages) | ||
| throws JMSException { | ||
| throw new AbstractMethodError( | ||
| "JMS 1.0 provider does not implement createConnectionConsumer(Destination, ...)"); | ||
| } | ||
|
|
||
| @Override | ||
| public ConnectionConsumer createConnectionConsumer( | ||
| Queue queue, String messageSelector, ServerSessionPool sessionPool, int maxMessages) | ||
| throws JMSException { | ||
| return delegate.createConnectionConsumer(queue, messageSelector, sessionPool, maxMessages); | ||
| } | ||
|
|
||
| @Override | ||
| public ConnectionConsumer createConnectionConsumer( | ||
| Topic topic, String messageSelector, ServerSessionPool sessionPool, int maxMessages) | ||
| throws JMSException { | ||
| return delegate.createConnectionConsumer(topic, messageSelector, sessionPool, maxMessages); | ||
| } | ||
|
|
||
| @Override | ||
| public ConnectionConsumer createDurableConnectionConsumer( | ||
| Topic topic, | ||
| String subscriptionName, | ||
| String messageSelector, | ||
| ServerSessionPool sessionPool, | ||
| int maxMessages) | ||
| throws JMSException { | ||
| return delegate.createDurableConnectionConsumer( | ||
| topic, subscriptionName, messageSelector, sessionPool, maxMessages); | ||
| } | ||
| } |
61 changes: 61 additions & 0 deletions
61
...rumentation/jms/javax-jms-1.1/src/testFixtures/java/jms10mock/Jms10ConnectionFactory.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package jms10mock; | ||
|
|
||
| import javax.jms.Connection; | ||
| import javax.jms.ConnectionFactory; | ||
| import javax.jms.JMSException; | ||
| import javax.jms.QueueConnection; | ||
| import javax.jms.QueueConnectionFactory; | ||
| import javax.jms.TopicConnection; | ||
| import javax.jms.TopicConnectionFactory; | ||
|
|
||
| /** | ||
| * Wraps a real {@link ConnectionFactory} but simulates a JMS 1.0 provider. | ||
| * | ||
| * <p>In JMS 1.0, clients used the domain-specific {@link QueueConnectionFactory} and {@link | ||
| * TopicConnectionFactory} to obtain connections. The unified {@link ConnectionFactory} and its | ||
| * {@code createConnection()} methods are JMS 1.1 additions that this wrapper does not support. | ||
| */ | ||
| public class Jms10ConnectionFactory implements QueueConnectionFactory, TopicConnectionFactory { | ||
| private final ConnectionFactory delegate; | ||
|
|
||
| public Jms10ConnectionFactory(ConnectionFactory delegate) { | ||
| this.delegate = delegate; | ||
| } | ||
|
|
||
| // --- JMS 1.1-only unified ConnectionFactory methods --- | ||
|
|
||
| @Override | ||
| public Connection createConnection() throws JMSException { | ||
| return delegate.createConnection(); | ||
| } | ||
|
|
||
| @Override | ||
| public Connection createConnection(String userName, String password) throws JMSException { | ||
| return delegate.createConnection(userName, password); | ||
| } | ||
|
|
||
| // --- JMS 1.0 QueueConnectionFactory methods --- | ||
| @Override | ||
| public QueueConnection createQueueConnection() throws JMSException { | ||
| return new Jms10Connection(delegate.createConnection()); | ||
| } | ||
|
|
||
| @Override | ||
| public QueueConnection createQueueConnection(String userName, String password) | ||
| throws JMSException { | ||
| return new Jms10Connection(delegate.createConnection(userName, password)); | ||
| } | ||
|
|
||
| // --- JMS 1.0 TopicConnectionFactory methods --- | ||
|
|
||
| @Override | ||
| public TopicConnection createTopicConnection() throws JMSException { | ||
| return new Jms10Connection(delegate.createConnection()); | ||
| } | ||
|
|
||
| @Override | ||
| public TopicConnection createTopicConnection(String userName, String password) | ||
| throws JMSException { | ||
| return new Jms10Connection(delegate.createConnection(userName, password)); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a JMS 1.0 provider uses
QueueConnection.createQueueSession/TopicConnection.createTopicSessionwithtransacted=trueorSession.CLIENT_ACKNOWLEDGE,getAcknowledgeMode()is exactly the 1.1 method that raisesAbstractMethodError, so this fallback records theSessionStateas AUTO_ACKNOWLEDGE.JMSMessageConsumerInstrumentationonly defers spans toacknowledge()/commit()whenSessionStatereports client-ack/transacted, and theCommit/Recoveradvices have the same guards, so these JMS 1.0 sessions now finish spans and time-in-queue batches with auto-ack semantics instead of the session's actual semantics. The fallback should preserve at least the transacted/client-ack mode from creation orgetTransacted()rather than defaulting every error to AUTO.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been done on purpose - the tradeoff is to have not exact time here but still having 1.0 supported. Otherwise I would have needed to instrument also
javax.jms.Connection(and all the subclasses) to capture that info when createSession/createTopicSession/createQueueSession are called. And that would have been made the instrumentation way heavier just to support few library versions. The tradeoff that we have in this PR is to me acceptable