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.

Sign up for GitHub

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

Jump to bottom

[FullStory] Update snippet #153570

Merged
merged 3 commits into from
Apr 18, 2023
Merged

[FullStory] Update snippet #153570

merged 3 commits into from
Apr 18, 2023

Conversation

afharo
Copy link
Member

@afharo afharo commented Mar 23, 2023 •
edited by kibanamachine
Loading

Summary

We recently noticed that we are running a fairly old version of FullStory that wouldn't allow us to extend the context of the collected stats via the FS.setVars API.

This PR updates the snippet to the latest version of https://edge.fullstory.com/s/fs.js.

For maintainers

@afharo afharo added Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Journey/Onboarding Platform Journey Onboarding team labels Mar 23, 2023
@afharo afharo requested review from shahinakmal and a team March 23, 2023 16:00
@afharo afharo self-assigned this Mar 23, 2023
afharo
afharo commented Mar 23, 2023
View reviewed changes
x-pack/plugins/cloud_integrations/cloud_full_story/server/assets/fullstory_library.js Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the real important change in this PR.

packages/analytics/shippers/fullstory/src/load_snippet.ts
@@ -36,7 +36,7 @@ export interface FullStorySnippetConfig {
}

export function loadSnippet({
scriptUrl = 'edge.fullstory.com/s/fs.js',
scriptUrl = 'https://edge.fullstory.com/s/fs.js',
Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed that it wouldn't load correctly without the https:// if scriptUrl is not provided (but Kibana does provide it, so it's more for the sake of correctness rather than anything else).

@afharo afharo requested a review from a team March 23, 2023 18:21
@afharo afharo marked this pull request as ready for review March 23, 2023 18:21
@afharo afharo requested a review from a team as a code owner March 23, 2023 18:21
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-onboarding (Team:Journey/Onboarding)

@lukeelmers
Copy link
Member

IIRC, the original snippet was reviewed by infosec prior to merging into the repo. Should we do the same here?

cc @elastic/kibana-security

alisonelizabeth
alisonelizabeth approved these changes Mar 24, 2023
View reviewed changes
Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @afharo!

@azasypkin
Copy link
Member

IIRC, the original snippet was reviewed by infosec prior to merging into the repo. Should we do the same here?
cc https://github.com/orgs/elastic/teams/kibana-security

Thanks for the ping! I'm not sure if warrants a re-review from the InfoSec (we should ask though!), but I think we definitely need SBOM (list of the 3rd-party dependencies with versions bundled into this library) for this upgrade. I've been told that InfoSec team might help with that.

@afharo
Copy link
Member Author

afharo commented Apr 18, 2023

@elasticmachine merge upstream

@afharo afharo enabled auto-merge (squash) April 18, 2023 10:17
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cloudFullStory 4.3KB 4.3KB +8.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 432 435 +3

Total ESLint disabled count

id before after diff
securitySolution 512 515 +3

History

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

cc @afharo

@afharo afharo merged commit 7de4733 into elastic:main Apr 18, 2023
@afharo afharo deleted the fullstory/update-snippet branch April 18, 2023 11:06
@kibanamachine kibanamachine mentioned this pull request Apr 18, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 18, 2023
@afharo
[FullStory] Update snippet (elastic#153570)
05d59c3 
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 7de4733)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.7

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@legrego
Copy link
Member

legrego commented Apr 18, 2023

@afharo Would it make sense to also backport this to 7.17?

kibanamachine added a commit that referenced this pull request Apr 18, 2023
@kibanamachine @afharo
[8.7] [FullStory] Update snippet (#153570) (#155138)
73d9e92 
# Backport

This will backport the following commits from `main` to `8.7`:
- [[FullStory] Update snippet
(#153570)](#153570)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alejandro Fernández
Haro","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-04-18T11:06:05Z","message":"[FullStory]
Update snippet (#153570)\n\nCo-authored-by: kibanamachine
<[email protected]>","sha":"7de4733f7255c976bf6503669e31e939cb9f485f","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Telemetry","release_note:skip","backport:prev-minor","Team:Journey/Onboarding","v8.8.0"],"number":153570,"url":"https://github.com/elastic/kibana/pull/153570","mergeCommit":{"message":"[FullStory]
Update snippet (#153570)\n\nCo-authored-by: kibanamachine
<[email protected]>","sha":"7de4733f7255c976bf6503669e31e939cb9f485f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/153570","number":153570,"mergeCommit":{"message":"[FullStory]
Update snippet (#153570)\n\nCo-authored-by: kibanamachine
<[email protected]>","sha":"7de4733f7255c976bf6503669e31e939cb9f485f"}}]}]
BACKPORT-->

Co-authored-by: Alejandro Fernández Haro <[email protected]>
rylnd added a commit that referenced this pull request Apr 21, 2023
@rylnd
Merge branch '8.7' into risk_score_api_8.7
c70c7ea 
* 8.7: (93 commits)
  [8.7] [Controls] Use EUI Selectable for Field search (#151231) (#155454)
  [8.7] [Synthetics] Fix  performance breakdown link from error details page (#155393) (#155427)
  [8.7] [DOCS] Remove or move book-scoped attributes (#155210) (#155426)
  [8.7] [Synthetics] add default email recovery message (#154862) (#155418)
  [8.7] [Uptime] Add both both ip filters for view host in uptime location for host and monitor (#155382) (#155399)
  [8.7] Setup Node.js environment before instrumenting Kibana with APM. (#155063) (#155300)
  [8.7] [Discover] Address react warnings for legacy table (#154579) (#155345)
  [8.7] [Fleet] Fix logs useless rerender (#155305) (#155310)
  [8.7] [kbn-failed-test-reporter-cli] truncate report message to fix github api call failure (#155141) (#155286)
  [8.7][APM] Fleet migration support for bundled APM package (#153159)  (#155281)
  [8.7] [Enterprise Search] Fix Connector scheduling show week information correctly (#155191) (#155227)
  [8.7] [Synthetics] Fix pending count in case of location filtering (#155200) (#155225)
  [8.7] [Controls] Add Expensive Queries Fallback (#155082) (#155189)
  [8.7] [data view field editor] Runtime field code editor - move state out of controller (#155107) (#155150)
  [8.7] [FullStory] Update snippet (#153570) (#155138)
  [8.7] [Security Solution][Exceptions] - Fix exception operator logic when mapping conflict (#155071) (#155094)
  [DOCS] Adds 8.7.1 release notes (#154844)
  [8.7] Sync bundled packages with Package Storage (#155042)
  [APM] plugin description (#154811)
  Update api.asciidoc (#155021)
  ...
@azasypkin
Copy link
Member

@afharo Would it make sense to also backport this to 7.17?

@afharo @legrego did you eventually agree not to do this, or did it just go under the radar?

@legrego
Copy link
Member

legrego commented May 31, 2023

@afharo Would it make sense to also backport this to 7.17?

@afharo @legrego did you eventually agree not to do this, or did it just go under the radar?

Fell off my radar. @afharo, can we backport this?

@afharo
Copy link
Member Author

afharo commented Jun 1, 2023

@legrego @azasypkin, apologies! I missed that request as well.

The affected API, FS.setVars, is not used in 7.17. That's why we didn't backport it in the first place. However, I can see the benefits of maintaining a common snippet in all active versions.

I'll trigger the backport now.

afharo added a commit to afharo/kibana that referenced this pull request Jun 1, 2023
@afharo
[FullStory] Update snippet (elastic#153570)
0e8f0dc 
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 7de4733)

# Conflicts:
#	NOTICE.txt
#	packages/analytics/shippers/fullstory/src/load_snippet.ts
#	x-pack/plugins/cloud/server/assets/fullstory_library.js
@afharo afharo mentioned this pull request Jun 1, 2023
Merged
@afharo
Copy link
Member Author

afharo commented Jun 1, 2023

💚 All backports created successfully

Status Branch Result
✅ 7.17

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

afharo added a commit that referenced this pull request Jun 1, 2023
@afharo
[7.17] [FullStory] Update snippet (#153570) (#158805)
1643419 
# Backport

This will backport the following commits from `main` to `7.17`:
- [[FullStory] Update snippet
(#153570)](#153570)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alejandro Fernández
Haro","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-04-18T11:06:05Z","message":"[FullStory]
Update snippet (#153570)\n\nCo-authored-by: kibanamachine
<[email protected]>","sha":"7de4733f7255c976bf6503669e31e939cb9f485f","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Telemetry","release_note:skip","backport:prev-minor","Team:Journey/Onboarding","v8.8.0","v8.7.1"],"number":153570,"url":"https://github.com/elastic/kibana/pull/153570","mergeCommit":{"message":"[FullStory]
Update snippet (#153570)\n\nCo-authored-by: kibanamachine
<[email protected]>","sha":"7de4733f7255c976bf6503669e31e939cb9f485f"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/153570","number":153570,"mergeCommit":{"message":"[FullStory]
Update snippet (#153570)\n\nCo-authored-by: kibanamachine
<[email protected]>","sha":"7de4733f7255c976bf6503669e31e939cb9f485f"}},{"branch":"8.7","label":"v8.7.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/155138","number":155138,"state":"MERGED","mergeCommit":{"sha":"73d9e928b0af0ab41728c396ff4eca55ef88fae5","message":"[8.7]
[FullStory] Update snippet (#153570) (#155138)\n\n# Backport\n\nThis
will backport the following commits from `main` to `8.7`:\n-
[[FullStory] Update
snippet\n(#153570)](https://github.com/elastic/kibana/pull/153570)\n\n<!---
Backport version: 8.9.7 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Alejandro
Fernández\nHaro\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2023-04-18T11:06:05Z\",\"message\":\"[FullStory]\nUpdate
snippet (#153570)\\n\\nCo-authored-by:
kibanamachine\n<[email protected]>\",\"sha\":\"7de4733f7255c976bf6503669e31e939cb9f485f\",\"branchLabelMapping\":{\"^v8.8.0$\":\"main\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"Feature:Telemetry\",\"release_note:skip\",\"backport:prev-minor\",\"Team:Journey/Onboarding\",\"v8.8.0\"],\"number\":153570,\"url\":\"https://github.com/elastic/kibana/pull/153570\",\"mergeCommit\":{\"message\":\"[FullStory]\nUpdate
snippet (#153570)\\n\\nCo-authored-by:
kibanamachine\n<[email protected]>\",\"sha\":\"7de4733f7255c976bf6503669e31e939cb9f485f\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v8.8.0\",\"labelRegex\":\"^v8.8.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/153570\",\"number\":153570,\"mergeCommit\":{\"message\":\"[FullStory]\nUpdate
snippet (#153570)\\n\\nCo-authored-by:
kibanamachine\n<[email protected]>\",\"sha\":\"7de4733f7255c976bf6503669e31e939cb9f485f\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Alejandro Fernández Haro <[email protected]>"}}]}] BACKPORT-->
@afharo afharo mentioned this pull request Aug 6, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shahinakmal shahinakmal shahinakmal approved these changes

@alisonelizabeth alisonelizabeth alisonelizabeth approved these changes

@pgayvallet pgayvallet pgayvallet approved these changes

Assignees

@afharo afharo

Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes Team:Journey/Onboarding Platform Journey Onboarding team v7.17.11 v8.7.1 v8.8.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@afharo @elasticmachine @lukeelmers @azasypkin @kibana-ci @kibanamachine @legrego @pgayvallet @alisonelizabeth @shahinakmal