-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[receiver/azureeventhub] remove continue from timestamp parse failure #31245
Conversation
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.
LGTM
@nslaughter - If this is standard practice elsewhere, I think it makes sense. I intentionally skipped these records because they are really hard to find in Splunk. Maybe it makes sense to add a warning? I'll look at other receivers and see what they do. What do you think @atoulme? |
per comments of @cparkins Changed behavior so we set Added the option to IgnoreObservedTimestamp in test as we now set the observed timestamp to code observed time, per field description in logs data model; however that breaks test expectation that ALL fields match test record. And noted I need to stop force pushing, so I don't disappear the comments. |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
hi @dashpole, I have an approving review from a codeowner. However, this may not show it as he wasn't a codeowner when the PR began? ... So I'm pinging. Hoping you can help getting this merged as assignee. Thank you! |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
I'd like to see these changes get integrated. Can we get this reopened? I believe we have users that are experiencing this issue and it would be good to get more information. I believe these changes are good in general anyway. @atoulme - Can you open this back up? |
Description:
Link to tracking Issue:
Testing:
Analyzed log records with and without this change.
Documentation: