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

[citrix_adc] Make date/time format configurable #11258

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jrmolin
Copy link
Contributor

@jrmolin jrmolin commented Sep 26, 2024

Proposed commit message

[citrix_adc] Make date/time format configurable

  • User was seeing errors when Citrix appliance sent DeLink logs with Day/Month/Year on days larger than 12 (so obviously not a month).
  • Found a lot of places where the default date formatting chooses month first, which will cause inaccuracies in how the data is stored.
  • Created a new integration option that will allow the customer to set the specific format (dd/MM/yyyy:HH:mm:ss is what this customer would use).
  • Needed to find all the places where date information is parsed and make sure they are consistent.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

* Customer was seeing errors when Citrix appliance sent DeLink logs with
  Day/Month/Year on days larger than 12 (so obviously not a month).
* Found a lot of places where the default date formatting chooses month
  first, which will cause inaccuracies in how the data is stored.
* Created a new integration option that will allow the customer to set
  the specific format (dd/MM/yyyy:HH:mm:ss is what this customer would
  use).
* Needed to find all the places where date information is parsed and
  make sure they are consistent.
@jrmolin jrmolin added enhancement New feature or request bugfix Pull request that fixes a bug issue labels Sep 26, 2024
@jrmolin jrmolin requested review from a team as code owners September 26, 2024 13:46
@jrmolin jrmolin marked this pull request as draft September 26, 2024 14:19
@andrewkroh andrewkroh changed the title [citrix_adc] Allow customer to define date/time format [citrix_adc] Make date/time format configurable Sep 27, 2024
packages/citrix_adc/changelog.yml Outdated Show resolved Hide resolved
default: ''
description: |-
Format to use to parse the date/time fields in the data.
If not populated, this uses one of "yyyy/MM/dd:HH:mm:ss", "MM/dd/yyyy:HH:mm:ss", or the ISO8601 standard.
Copy link
Member

@andrewkroh andrewkroh Sep 27, 2024

Choose a reason for hiding this comment

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

As a user, how do I learn what the available format specifiers are? I think we need to link to the relevant documentation or explicitly state what we are using to implement parsing so that the user can find the information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, link to the formats allowed by the Java class I'm using. So https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/time/format/DateTimeFormatter.html ?

Copy link
Member

Choose a reason for hiding this comment

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

The ES docs link out to DateTimeFormatter so I think that's what this should link users to as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, found them in the Painless documentation: https://www.elastic.co/guide/en/elasticsearch/painless/current/painless-datetime.html. Looks exactly like what I'm trying to do. Should I fall back to the default parsing options?

@andrewkroh andrewkroh removed the bugfix Pull request that fixes a bug issue label Sep 27, 2024
@elastic-vault-github-plugin-prod

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@jrmolin jrmolin marked this pull request as ready for review September 30, 2024 20:17
@andrewkroh andrewkroh added the Team:Security-Deployment and Devices Deployment and Devices Security team [elastic/sec-deployment-and-devices] label Sep 30, 2024
@elasticmachine
Copy link

Pinging @elastic/sec-deployment-and-devices (Team:Security-Deployment and Devices)

@elasticmachine
Copy link

💚 Build Succeeded

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:citrix_adc Citrix ADC Team:Security-Deployment and Devices Deployment and Devices Security team [elastic/sec-deployment-and-devices]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants