Skip to content

Fix duplicate get probe by id api calls in traceroute visualizer #941

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

Conversation

OmPals
Copy link
Contributor

@OmPals OmPals commented Mar 20, 2025

Description

In this PR, I have fixed an includes check over allProbes array ref.
The probeData.prb_id is a number and the code stores it in allProbes array as a string.
So, while checking if the probe_id exists in allProbes array, we need to convert it to string first while comparing.

TracerouteMonitor component was storing duplicate probe ids in this ref: allProbes.
Expected behavior is to skip adding the probe id in the allProbes array.

Problem it solves is:
In this if block, there is an API call to RIPE Atlas to fetch the probe by ID. Since the bug allowed duplicate probe ids, the API requests were also made for duplicate probe ids. Which are redundant calls.

Related issue

How Has This Been Tested?

I observed this traceroute: 75404443.
Total 19 probes reached the target. Since, it should fetch only 19 probes data. So, it should only call 19 times to the https://atlas.ripe.net/api/v2/probes/ endpoint.

Tools used for testing:

Chrome devtool's network tab and console.

Testing strategy

  1. Observe this behavior on the IHR live website. Result: 49 requests for 19 probes.
  2. Check if the fix works in the local implementation. Result: 19 requests for 19 probes.

Before fix: 49 requests for 19 probes (duplicate requests)

image

After fix: 19 requests for 19 probes. (1 request for 1 probe)

image

Affected areas

This ref is used as a prop in TracerouteProbesTable.
In this component, there's a computed ref called paginatedProbes, in which there's a set variable called uniqueProbes. It makes sure that a probe is not duplicated by adding from allProbes array into uniqueProbes set.
Since, I have removed the duplicates from the source itself, this Set is not required.
TracerouteProbesTable is only used by TracerouteMonitor component.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@dpgiakatos
Copy link
Member

Violation of contribution guidelines:
PRs from contributors who are not assigned to an issue will be automatically closed.

@dpgiakatos dpgiakatos closed this Mar 21, 2025
@dpgiakatos
Copy link
Member

Nice catch though. Please comply with the contribution guidelines by reporting this as an issue first.

@OmPals
Copy link
Contributor Author

OmPals commented Mar 21, 2025

Sure.

@OmPals
Copy link
Contributor Author

OmPals commented Mar 21, 2025

Hello @dpgiakatos : Created the issue: #942

dpgiakatos pushed a commit that referenced this pull request Mar 25, 2025
* fix duplicate get probe by id api calls in traceroute visualizer

* add nodeSet ref to discard duplicate resource calls

* remove unnecessary set for filtering unique probes from allProbes array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants