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

Support timezone param for WebSocketTransport execute #414

Merged

Conversation

nishantagarwal1990
Copy link
Contributor

@nishantagarwal1990 nishantagarwal1990 commented Dec 14, 2023

The SignaFlow client for java does not have a way to specify the timezone while the signalfx-python lib does. We would like to have similar capabilities.

Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

This seems fine to me, but I'd like to see two things:

  1. Can you add a link to the docs for the timezone parameter? Is is just passed over the wire to the receiving side or is this actually configuring something in the transport itself?
  2. How would a user know what kind of value to pass in there? Do they pass in PDT or do they pass in America/Los_Angeles or would either work?

@nishantagarwal1990
Copy link
Contributor Author

The timezone is passed over the wire to the receiving side as a query parameter. The docs link is https://dev.splunk.com/observability/docs/signalflow/#Supported-SignalFlow-time-zones

The supported timezones are https://dev.splunk.com/observability/docs/time_zone

@breedx-splk
Copy link
Contributor

Sorry what I meant was can you please add that in the javadocs so that a user can see it? 👍🏻

@breedx-splk breedx-splk merged commit af0910e into signalfx:main Dec 15, 2023
@nishantagarwal1990 nishantagarwal1990 deleted the na/add-timezone-param-#413 branch December 15, 2023 17:06
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