Skip to content

[23573] Using original writer in statistics when possible#5977

Merged
rsanchez15 merged 4 commits intomasterfrom
feature/og-writer-in-statistics
Sep 3, 2025
Merged

[23573] Using original writer in statistics when possible#5977
rsanchez15 merged 4 commits intomasterfrom
feature/og-writer-in-statistics

Conversation

@emiliocuestaf
Copy link
Copy Markdown
Contributor

@emiliocuestaf emiliocuestaf commented Aug 6, 2025 •
edited by cferreiragonz
Loading

Description

This PR adds the code to use the original writer to compute statistics when available. If it is not available, the statistics remain computed using the current writer GUID.

This PR depends on #5975 and must be merged after that one.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • N/A Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • N/A Any new/modified methods have been properly documented using Doxygen.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • N/A Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • N/A Changes are API compatible.
  • N/A New feature has been added to the versions.md file (if applicable).
  • N/A New feature has been documented/Current behavior is correctly described in the documentation.
  • N/A Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • N/A If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@github-actions github-actions bot added the ci-pending PR which CI is running label Aug 6, 2025
@juanlofer-eprosima juanlofer-eprosima added this to the v3.4.0 milestone Aug 12, 2025
juanlofer-eprosima
juanlofer-eprosima reviewed Aug 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Changes LGTM, I would maybe change the PR title to something more specific (e.g. Use original writer info in latency calculation), and please better use present tense. Will rereview after #5975 is merged.

@emiliocuestaf emiliocuestaf changed the base branch from master to feature/add_original_writer_parameter August 28, 2025 08:28
@emiliocuestaf emiliocuestaf force-pushed the feature/og-writer-in-statistics branch from 5e6dca7 to 338d1f4 Compare August 28, 2025 08:31
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 28, 2025 •
edited
Loading

🧪 CI Insights

Here's what we observed from your CI run for f477735.

❌ Failed Jobs

Pipeline Job Health on master Retries 🔍 CI Insights 📄 Logs
Fast DDS MacOS CI mac-ci / reusable-mac-ci (RelWithDebInfo) Unknown 0 View View

Base automatically changed from feature/add_original_writer_parameter to master August 29, 2025 07:14
@emiliocuestaf
Adding logic to compute statistics with the original writer
680d698 
Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
@emiliocuestaf
Refs #23573, reflecting changes in original writer info struct
5822866 
Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
@emiliocuestaf emiliocuestaf force-pushed the feature/og-writer-in-statistics branch from 338d1f4 to 5822866 Compare August 29, 2025 07:41
@emiliocuestaf emiliocuestaf force-pushed the feature/og-writer-in-statistics branch from 63cf79d to 63b786a Compare August 29, 2025 08:23
@emiliocuestaf
Uncrustify
ebc7ad2 
Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
@emiliocuestaf
Remove unwanted file
f477735 
Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
@emiliocuestaf emiliocuestaf force-pushed the feature/og-writer-in-statistics branch from 63b786a to f477735 Compare August 29, 2025 08:27
cferreiragonz
cferreiragonz approved these changes Aug 29, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@cferreiragonz cferreiragonz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Tested with: https://github.com/eProsima/DDS-Router/actions/runs/17291261488

@cferreiragonz cferreiragonz requested review from richiprosima and removed request for richiprosima August 29, 2025 08:28
@rsanchez15 rsanchez15 merged commit 7830104 into master Sep 3, 2025
34 of 38 checks passed
@rsanchez15 rsanchez15 deleted the feature/og-writer-in-statistics branch September 3, 2025 06:12
MiguelCompany pushed a commit that referenced this pull request Sep 4, 2025
@emiliocuestaf @MiguelCompany
Using original writer in statistics when possible (#5977)
982fca6 
* Adding logic to compute statistics with the original writer

Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>

* Refs #23573, reflecting changes in original writer info struct

Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>

* Uncrustify

Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>

* Remove unwanted file

Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>

---------

Signed-off-by: Emilio Cuesta <emiliocuesta@eprosima.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juanlofer-eprosima juanlofer-eprosima juanlofer-eprosima left review comments

@cferreiragonz cferreiragonz cferreiragonz approved these changes

@richiprosima richiprosima Awaiting requested review from richiprosima

Assignees

No one assigned

Labels

ci-pending PR which CI is running

Projects

None yet

Milestone

v3.4.0 - Frozen ❄️

Development

Successfully merging this pull request may close these issues.

4 participants

@emiliocuestaf @cferreiragonz @juanlofer-eprosima @rsanchez15