Skip to content
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

NMS-12242: Invalid timestamps used for CollectionSets generated by telemetryd #2668

Merged
merged 2 commits into from Aug 29, 2019

Conversation

mattixtech
Copy link
Contributor

@mattixtech mattixtech commented Aug 23, 2019

This PR updates telemetry processing to use the collected time included in a telemetry packet as the timestamp rather than System.currentTimeMillis(). Previously we could end up with a time that was incorrect because the message could have been sitting in Kafka for a long time for instance.

Now we only use the current time if we couldn't find a timestamp in the packet itself.

I couldn't find a convenient spot to add test coverage for this, however I did verify that it is getting set correctly by tracing an IT.

All Contributors

  • Have you read and followed our Contribution Guidelines?
  • Have you made an issue in the OpenNMS issue tracker?
    If so, you should:
    1. update the title of this PR to be of the format: ${JIRA-ISSUE-NUMBER}: subject of pull request
    2. update the JIRA link at the bottom of this comment to refer to the real issue number
    3. prefix your commit messages with the issue number, if possible
  • Have you made a comment in that issue which points back to this PR?
  • Have you updated the JIRA link at the bottom of this comment to link to your issue?
  • If this is a new or updated feature, is there documentation for the new behavior?
  • [] If this is new code, are there unit and/or integration tests?
  • If this PR targets a foundation-* branch, does it avoid changing files in $OPENNMS_HOME/etc/?

Pull Request Process

One or more reviewers should be assigned to each PR.

If you know that a particular person is subject matter expert in the area your PR affects, feel free to assign one or more reviewers when you create this PR, otherwise reviewers will be assigned for you.

Once the reviewer(s) accept the PR and the branch passes continuous integration in Bamboo, the PR is eligible for merge.

At that time, if you have commit access (are an OpenNMS Group employee or a member of the Order of the Green Polo) you are welcome to merge the PR.
Otherwise, a reviewer can merge it for you.

Thanks for taking time to contribute!

External References

@pbrane
Copy link

pbrane commented Aug 23, 2019

now() is also problematic because I’m assuming you’re referring to the DB func and that call assumes the DB system time is inline with the NMS and “reality.”

@mattixtech
Copy link
Contributor Author

now() is also problematic because I’m assuming you’re referring to the DB func and that call assumes the DB system time is inline with the NMS and “reality.”

Oops that was misleading... I'll update the PR description. It is in fact using the current time via the JVM to set the timestamp if we couldn't find one in the packet.

@mattixtech
Copy link
Contributor Author

@cgorantla Hey Chandra, can you take a look at this one? It is pretty trivial, just making use of the timestamp value that was already being collected by passing it down into the collection set. Thanks.

@mattixtech mattixtech merged commit 3229d70 into release-25.0.0 Aug 29, 2019
@mattixtech mattixtech deleted the mbrooks/NMS-12242 branch August 29, 2019 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants