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

Add s2s and user sync features to roxot analytics adapter #1337

Merged
merged 35 commits into from
Jul 10, 2017

Conversation

sergey-roxot
Copy link
Contributor

@sergey-roxot sergey-roxot commented Jun 29, 2017

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other

Description of change

Added features to roxot analytics adapter:

  • Tracking bidders using Prebid Server by adding '(s2s)' postfix to bidder code.
  • Setting cookie uid to visitors of sites with enabled roxot analytics adapter.

@mkendall07
Copy link
Member

Nice! I'm adding a src attribute to the existing bids. Would you be able to use that instead of piping the whole configuration object?

src: CONSTANTS.S2S.SRC

@sergey-roxot
Copy link
Contributor Author

src attribute is good. I'm going to check it right now and fix our feature after src improvement will be in master branch.

@sergey-roxot
Copy link
Contributor Author

When can we expect #1330 to be merged to the master?

@mkendall07
Copy link
Member

@sergey-roxot
Just merged.

@sergey-roxot
Copy link
Contributor Author

We had removed s2s feature and now using source argument. Our pull request includes only user sync feature now.

@sergey-roxot
Copy link
Contributor Author

When do you plan to review our pull request?

(result) => utils.logInfo('Event ' + eventType + ' sent ' + sendDataType + ' to roxot prebid analytic with result' + result),
JSON.stringify(data)
);
let xhr = new XMLHttpRequest();
Copy link
Member

@mkendall07 mkendall07 Jul 10, 2017

Choose a reason for hiding this comment

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

what's the reason you can't use ajax function?

Copy link
Member

@mkendall07 mkendall07 Jul 10, 2017

Choose a reason for hiding this comment

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

FYI, you can turn on cookies:

ajax(ENDPOINT, handleResponse, payload, {
        contentType: 'text/plain',
        withCredentials: true
      });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Depending on the publisher's setup, some requests are processed longer than others. But when it takes more than 3 sec to record all events, requests are canceled by the recently implemented timeout to ajax requests. As a result, we may lose some of the data. While optimizing our system and speeding up data processing, we want to make sure nothing is lost.

Copy link
Member

Choose a reason for hiding this comment

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

Fair point. We should allow analytics adapters to override the timeout. For now this is fine.

@mkendall07 mkendall07 merged commit b54004d into prebid:master Jul 10, 2017
dbemiller added a commit that referenced this pull request Jul 10, 2017
jbAdyoulike pushed a commit to jbAdyoulike/Prebid.js that referenced this pull request Sep 21, 2017
* add roxot adapter

* update roxot analytics system

* remove unused test

* remove unused import

* update code after review

* Added s2s tracking

* Add user sync

* Empty s2sConfig case handling added

* Removed s2s

* Add s2s bidder code setters

* Revert "Add s2s bidder code setters"

This reverts commit 89a44e3.

* Revert "Removed s2s"

This reverts commit 929707f.

* Revert "Empty s2sConfig case handling added"

This reverts commit 2f55e24.

* Revert s2s

* Removed user sync feature via iframe

* Add ajax withCredentials option

* Add xhr withCredentials
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* add roxot adapter

* update roxot analytics system

* remove unused test

* remove unused import

* update code after review

* Added s2s tracking

* Add user sync

* Empty s2sConfig case handling added

* Removed s2s

* Add s2s bidder code setters

* Revert "Add s2s bidder code setters"

This reverts commit 89a44e3.

* Revert "Removed s2s"

This reverts commit 929707f.

* Revert "Empty s2sConfig case handling added"

This reverts commit 2f55e24.

* Revert s2s

* Removed user sync feature via iframe

* Add ajax withCredentials option

* Add xhr withCredentials
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.

4 participants