-
Notifications
You must be signed in to change notification settings - Fork 113
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
Accoil analytics destination #3873
base: develop
Are you sure you want to change the base?
Accoil analytics destination #3873
Conversation
Thank you @accoilmj for contributing this PR. |
d516a61
to
7b8cc26
Compare
I signed and submitted this form but it reported an error it was unable to update the PR status:
|
Hi @manish339k. Thanks for taking the time to review. This wasn't really ready for review yet, it was just what I threw together to try and get it working locally but it won't run for me so I uploaded this in the hopes maybe something stood out. I raised this in the Slack workspace: https://rudderstack.slack.com/archives/C026KTP7KPW/p1731403790462469 When I try to run this locally I just see this anytime I call it:
Would any of the items you pointed out above potentially be the cause? |
7b8cc26
to
0d0570f
Compare
I've pushed fixes for most of these items but it still doesn't seem to run locally |
@accoilmj Can you please check if you are sending
|
Excellent @manish339k ! I wasn't setting that and now that I am it is working with a 200 and I can see my transformed JSON. Great. I can continue dev and testing on this now. Many thanks! Apologies if I missed that in the setup instructions |
2b3a9b6
to
0a933b0
Compare
@accoilmj, Could you please share the API documentation for Accoil analytics destination? |
@manish339k we don't really have a published API for it. At its core it simply expects Segment (or Segment like) formatted payloads to the endpoint https://in.accoil.com/segment. There is doc on our Segment integration and the types of calls we accept here: https://segment.com/docs/connections/destinations/catalog/actions-accoil-analytics/ |
e790f3f
to
5b571a9
Compare
5b571a9
to
3c4cd08
Compare
@accoilmj Could you please provide a test account so that we can validate and test the changes on our end? |
Hi @manish339k you should be able to sign up yourself at https://app.accoil.com/ just click 'Sign up' and fill in your details. After signing up you'll have a trial account and you can go to Settings > Account > General and copy the API key. |
@manish339k were you able to get in and get the key needed for testing? Let me know if I can help. |
@accoilmj I got the API key and am able to send the event. However, after sending the event, where can I verify the event delivery? Where should I check in the dashboard? I am seeing only one single page with API key after login. I have one more question - I'm getting a |
@manish339k the system should recognise the data was received within 30 minutes and take you into the product and stop showing you that setup page. After that you should be able to create a profile and see events. If it's taking longer than that then something is going wrong. I can look into where the events may be going. As for whether it accepts it without the authorization header that is expected. Currently favours throughput so it accepts all calls and processes it later in batches so unfortunately you can't necessarily tell that it will wind up being rejected. |
@manish339k it looks like data has started showing up so you should be able to get into the app now and click "Profiles" in the top nav then create a new profile to see the data. |
@accoilmj Thanks, I was missing the step of converting the API key to Base64 encoding. Now I am able to see the events in the dashboard. I will test, verify, and review the PR. |
Thanks @manish339k |
What are the changes introduced in this PR?
Adds a new destination for 'Accoil Analytics'.
I'm not sure what new readability format mentioned in the checklist refers to; please advise.
Please explain the objectives of your changes below
Add destination for 'Accoil analytics' so that it shows up in the UI for selection as a destination on sources in Rudderstack and sends events to our system.
All events go to a "/segment" API and we support: track, page, screen, identify, group. We do not support batch natively but I was hoping that the handling here where it loops over them and returns a separate request will work. Please advise if not.
Documentation PR is here: rudderlabs/rudderstack-docs#1196
UI PR is here: rudderlabs/rudder-integrations-config#1807
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A
Any new dependencies introduced with this change?
N/A
Any new generic utility introduced or modified. Please explain the changes.
N/A
Any technical or performance related pointers to consider with the change?
N/A
@coderabbitai review
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
Is the PR limited to 10 file changes?
Is the PR limited to one linear task?
Are relevant unit and component test-cases added in new readability format?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.