Skip to content

Conversation

@shahzad31
Copy link
Contributor

Summary

Fixes #77645

Some custom headers were set on order of ingest, which ended up changing on retries. This PR fixes that.

@shahzad31 shahzad31 requested a review from a team as a code owner September 22, 2020 15:39
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Sep 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@shahzad31 shahzad31 added v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 22, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

item.body = item.body.replace(
'"name":"client"',
'"name":"opbean-client-rum"'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

How risky is it to do a string replace? We might want to manipulate the body as an object rather than a string.

Comment on lines +73 to +94
function setItemMetaAndHeaders(item) {
const headers = {
'content-type': 'application/x-ndjson',
};

if (SECRET_TOKEN) {
headers.Authorization = `Bearer ${SECRET_TOKEN}`;
}

if (item.url === '/intake/v2/rum/events') {
if (iterIndex === userAgents.length) {
// set some event agent to opbean
setRumAgent(item);
iterIndex = 0;
}
headers['User-Agent'] = userAgents[iterIndex];
headers['X-Forwarded-For'] = userIps[iterIndex];
iterIndex++;
}
return headers;
}

Copy link
Member

Choose a reason for hiding this comment

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

IMO we ran into this situation because of side-effects. If we can make this (more) functional, i.e. don't mutate item and iterIndex, I feel like this code would be more robust and easier to understand. I think changing insertItem() to something like createRequestFromEvent() would help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, i will do it as a follow up PR, want to get this in to make sure it's working on other PRs.

@shahzad31 shahzad31 merged commit b3f605b into elastic:master Sep 23, 2020
@shahzad31 shahzad31 deleted the fix-csm-e2e-test branch September 23, 2020 07:06
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 23, 2020
* master: (31 commits)
  skip tests for old pacakge (elastic#78194)
  [Ingest Pipelines] Add url generator for ingest pipelines app (elastic#77872)
  [Lens] Rename "telemetry" to "stats" (elastic#78125)
  [CSM] Url search (elastic#77516)
  [Drilldowns] Config to disable URL Drilldown  (elastic#77887)
  [Lens] Combined histogram/range aggregation for numbers (elastic#76121)
  Remove legacy plugins support (elastic#77599)
  'Auto' interval must be correctly calculated for natural numbers (elastic#77995)
  [CSM] fix ingest data retry order messed up (elastic#78163)
  Add response status helpers (elastic#78006)
  Bump react-beautiful-dnd (elastic#78028)
  [Security Solution][Detection Engine] Bubbles up more error messages from ES queries to the UI (elastic#78004)
  Index pattern  - refactor constructor (elastic#77791)
  Add `xpack.security.sameSiteCookies` to docker allow list (elastic#78192)
  Remove [key: string]: any; from IIndexPattern (elastic#77968)
  Remove requirement for manage_index_templates privilege for Index Management (elastic#77377)
  [Ingest Manager] Agent bulk actions UI (elastic#77690)
  [Metrics UI] Add inventory view timeline (elastic#77804)
  Reporting/Docs: Updates for setting to enable CSV Download (elastic#78101)
  Update to latest rum-react (elastic#78193)
  ...
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 24, 2020
@sorenlouv sorenlouv removed the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CSM] E2E tests pass locally, fail on CI

6 participants