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

Fullstory V2 API Actions #1431

Merged
merged 3 commits into from
Aug 8, 2023
Merged

Conversation

mattgrosvenor-fs
Copy link
Contributor

Changes

Adding two new Actions to allow users to migrate from our V1 to V2 API

  • Track Events V2
  • Identify User V2

New actions are being created over changing existing actions to allow users to make the change when they want instead of being automatically switched.

Updated onDelete to use our new V2 API

This isn't really a callable action so little harm in automatically switching this call to V2.

Testing

Created unit tests for new functions/actions being added.
Ran everything through the local server and verified it came through correctly.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [Segmenters] Tested in the staging environment

@mattgrosvenor-fs mattgrosvenor-fs requested a review from a team as a code owner July 21, 2023 18:46
@mattgrosvenor-fs mattgrosvenor-fs requested a review from a team July 21, 2023 18:46
@marinhero
Copy link
Contributor

Hi Matt! this looks good to me, just a quick question. Is V2 ready on your end? because deploying this will start to render this to our mutual customers.

@joshrynneFS
Copy link

Hi @marinhero - jumping in while Matt is on vacation. Yes, our v2 APIs are ready and have been in-use by customers for a while now. We should be okay to deploy this change especially since the v2 actions are not enabled by default.

@marinhero
Copy link
Contributor

All right, marking this as ready for release. The next release train will launch on Tuesday August 2nd. Thank you for your help and patience. Let me know if you have any questions. Best.

@joshrynneFS
Copy link

@marinhero Are we on-track for deployment next week (Aug 2)? I still see an approval from @build-experience-team is needed before this can be merged?

@joe-ayoub-segment
Copy link
Contributor

Hi @joshrynneFS and @mattgrosvenor-fs ,

I'm just back from 4 weeks leave. My colleague @marinhero was kindly helping out with some PR reviews while I was out.
As you can imagine I've a few things to catch up on, including this PR.

I'm going to ask my team if there's a standard way for us to deploy the code in such a way so that the new Actions are hidden from customers until we've had the opportunity to test them properly.

Because of this I'm not sure that this PR will be deployed tomorrow. I'd also like to have the opportunity to review the code myself (unless @marinhero tells me that he's 100% happy with it).

Does the above sound OK to you @joshrynneFS and @mattgrosvenor-fs ?

Kind regards,
Joe

@joshrynneFS
Copy link

Welcome back @joe-ayoub-segment! When you say, "in such a way so that the new Actions are hidden from customers until we've had the opportunity to test them properly" does this mean more than just making them not installed by default? We were envisioning that customers would still be able to find them via the "New Mapping" button in our Cloud Mode destination.

It's okay if the changes don't make it into this week's deploy so that you perform a review. Do you think we could aim for next week's deploy?

Thanks,
Josh

@joe-ayoub-segment
Copy link
Contributor

Hi Josh,

By "in such a way so that the new Actions are hidden from customers until we've had the opportunity to test them properly" I meant that it would be desirable to be able to make these 2 new Actions available to just you and I in Production, and maybe one or two select customers. We could then ensure (via testing) that the new Actions work exactly as intended before other customers start to use them. Once customers start using an Action it makes it more complicated to change the code in those Actions. Does that make sense?

Cheers,
Joe

@joshrynneFS
Copy link

That does make sense, thanks. I didn't realize that was an option, but if there is a way to do this, we would love the opportunity to test it in a more controlled way.

@joe-ayoub-segment
Copy link
Contributor

Hi Josh,

I discussed this with the team and the only 2 potential options are:

  1. We add a feature flag to the UI to hide the new Actions from customers. This will require a change in our UI Repo and an App deploy. It's also not a repo I am familiar with yet.

  2. We deploy your changes to an internal Staging environment and then we do some robust testing. We could organize a Workshop for a couple of hours where we run some sample data through Staging. I can't give you access to the Staging environment as it is internal - but we could do a screenshare.

I'm leaning towards the second option as it should be simpler and should take less time! Keen to hear your thoughts.

Regards,
Joe

@joshrynneFS
Copy link

Hi Joe,

The second option seems okay with us. What kind of preparation would you need from us (sample data, environment setup, etc.) before the screenshare?

-Josh

@joe-ayoub-segment
Copy link
Contributor

I'd need to deploy your code to Staging, then schedule a call with you.

Do you have credentials which we can use when sending data from Segment to Fullstory (for the new Action)?
We can hand craft some events for testing purposes, so as long as you are clear on the type of events customers will forward to the new Actions then we should be good.

@joshrynneFS
Copy link

Great. You'll just need an API key for one of our sandboxes which we can provide on the call.

@joe-ayoub-segment
Copy link
Contributor

Deployed to Staging and did some testing with Nate and Josh.

V1 tests
image
image

V2 tests
image
image

Josh and Nate confirmed the updates were made on Fullstory.

Copy link
Contributor

@joe-ayoub-segment joe-ayoub-segment left a comment

Choose a reason for hiding this comment

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

Did testing in Staging for both legacy and new V2 Actions.

@joe-ayoub-segment
Copy link
Contributor

Hi @mattgrosvenor-fs ,
The flakey test has been fixed, however you'll need to get the latest code from main before the CI tests will pass for this PR.
Are you OK doing this please?

Best regards,
Joe

@mattgrosvenor-fs
Copy link
Contributor Author

Hi @joe-ayoub-segment ,
I updated our fork's main with latest segment main and rebased this branch on that. Hopefully tests pass now.

Best,
Matt

@joe-ayoub-segment joe-ayoub-segment merged commit 9c74977 into segmentio:main Aug 8, 2023
sayan-das-in pushed a commit that referenced this pull request Aug 9, 2023
* Add identifyUserV2 Action (#14)

* Create track event v2 action (#15)

* Update delete to use V2 (#16)
@joe-ayoub-segment
Copy link
Contributor

Hi @mattgrosvenor-fs , this PR has been deployed. Please check and let me know if everything looks good or not!

marinhero pushed a commit that referenced this pull request Aug 9, 2023
* Add identifyUserV2 Action (#14)

* Create track event v2 action (#15)

* Update delete to use V2 (#16)
@mattgrosvenor-fs
Copy link
Contributor Author

@joe-ayoub-segment , thank you! Every looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants