Skip to content

[#1843] [AMQ-9687] Add metrics about error and reconnect counts to network co…#1726

Open
mattrpav wants to merge 1 commit into
apache:mainfrom
mattrpav:AMQ-9687
Open

[#1843] [AMQ-9687] Add metrics about error and reconnect counts to network co…#1726
mattrpav wants to merge 1 commit into
apache:mainfrom
mattrpav:AMQ-9687

Conversation

@mattrpav

@mattrpav mattrpav commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

…nnector and network bridges

ref: [#1843]
ref: [AMQ-9687]

@mattrpav mattrpav self-assigned this Feb 25, 2026
@mattrpav mattrpav marked this pull request as draft February 25, 2026 15:18
@jbonofre jbonofre linked an issue Feb 25, 2026 that may be closed by this pull request
@mattrpav mattrpav marked this pull request as ready for review March 26, 2026 15:52
@mattrpav mattrpav changed the title [AMQ-9687] Add metrics about error and reconnect counts to network co… [#1843] [AMQ-9687] Add metrics about error and reconnect counts to network co… Mar 26, 2026
@mattrpav mattrpav moved this from Backlog to In progress in Apache ActiveMQ v6.3.0 May 3, 2026
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions Bot added the Stale label May 26, 2026
@github-actions github-actions Bot closed this Jun 1, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Done in Apache ActiveMQ v6.3.0 Jun 1, 2026
@mattrpav mattrpav removed the Stale label Jul 16, 2026
@mattrpav mattrpav reopened this Jul 16, 2026
@mattrpav mattrpav requested a review from cshannon July 16, 2026 13:58
@mattrpav mattrpav moved this from Done to In review in Apache ActiveMQ v6.3.0 Jul 16, 2026
private final ExecutorService syncExecutor = Executors.newSingleThreadExecutor();
private Transport duplexInboundLocalBroker = null;
private ProducerInfo duplexInboundLocalProducerInfo;
private AtomicLong startedTimestamp = new AtomicLong(0L);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
private AtomicLong startedTimestamp = new AtomicLong(0L);
private final AtomicLong startedTimestamp = new AtomicLong(0L);

// Once we have all required broker info we can attempt to start
// the local and then remote sides of the bridge.
doStartLocalAndRemoteBridges();
startedTimestamp.set(System.currentTimeMillis());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would move this to the start() method just because the other call to set back to 0 is in the stop() method so it makes more sense. I'd also add it to a try/finally probably. Here is what i mean.

Something like:

try {
    triggerStartAsyncNetworkBridgeCreation();
} catch (IOException e) {
    LOG.warn("Caught exception from remote start", e);
} finally {
    startedTimestamp.set(System.currentTimeMillis());
}

throw new IllegalStateException("You must configure the 'localURI' property");
}
LOG.info("Network Connector {} started", this);
bridgeExceptionCounter.setEnabled(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this not be true by default? I thought network statistics were optional and configurable?

public void resetStats(){
next.resetStats();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: remove extra line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Add additional observability fields to NetworkConnector

2 participants