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

Added call_stack_contains_unbacked field #308

Merged
merged 17 commits into from
Nov 8, 2022

Conversation

bit-envoy
Copy link
Contributor

Change Summary

Schema changes related to https://github.com/elastic/endpoint-dev/pull/11979

Release Target

8.6.0

@bit-envoy bit-envoy requested a review from a team as a code owner November 4, 2022 12:16
@bit-envoy bit-envoy self-assigned this Nov 4, 2022
@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 4, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-11-08T16:41:32.798+0000

  • Duration: 8 min 39 sec

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@jdu2600
Copy link
Contributor

jdu2600 commented Nov 7, 2022

I believe that you just need to add it under thread.Ext something like this -

- name: thread.Ext.call_stack_summary
level: custom
type: keyword

And these earlier lines handle making reusable and it valid at locations like Target.process.parent.thread.Ext -

reusable:
top_level: true
expected:
- Target
- { at: Target.process, as: parent }

@bit-envoy
Copy link
Contributor Author

I believe that you just need to add it under thread.Ext something like this -

- name: thread.Ext.call_stack_summary
level: custom
type: keyword

And these earlier lines handle making reusable and it valid at locations like Target.process.parent.thread.Ext -

reusable:
top_level: true
expected:
- Target
- { at: Target.process, as: parent }

Fixed, thanks.

@kevinlog
Copy link
Contributor

kevinlog commented Nov 7, 2022

@bit-envoy

Thanks for opening this PR! There are a few things to make sure you do in order to get a green build on this.

  1. You've added a new field to custom_schemas, but you need to make sure you add it to custom_subsets. Based on the README change here, it looks like this is to be a new field in alerts. You likely need to make a change to one of the custom_subsets here
  2. After you make the changes, run make to ensure the same changes show up in package/endpoint/data_stream. Your make will add fields to alerts data_stream I am assuming. Be sure to commit these changes in your PR.
  3. Add sample data for your change here, package/endpoint/data_stream/<streamname>/sample_event.json. This helps us document the intent behind package changes. In addition, after you update with sample data, CI will effectively use it as a unit test and check to see that the sample data you add matches the mapping changes you make.

I think this recent change has examples of what's described above.

Let me know of any questions you have

@bit-envoy
Copy link
Contributor Author

bit-envoy commented Nov 8, 2022

Thanks @kevinlog for tips. I have updated custom_subsets, ran make successfully, but it seems there are still some errors on build. I don't see why. Also when I ran make it does not add anything to data_stream. I added changes manually. Is there any clear indication what files should be edited manually and what files should be generated?

bit-envoy and others added 3 commits November 8, 2022 16:24
Co-authored-by: Gabriel Landau <42078554+gabriellandau@users.noreply.github.com>
@kevinlog
Copy link
Contributor

kevinlog commented Nov 8, 2022

@bit-envoy

I have updated custom_subsets, ran make successfully, but it seems there are still some errors on build. I don't see why. Also when I ran make it does not add anything to data_stream.

I'll check out your PR and take a look to see if I see anything

Copy link
Contributor

@kevinlog kevinlog 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 for the changes!

@bit-envoy bit-envoy merged commit 87e047f into master Nov 8, 2022
@elasticmachine
Copy link
Contributor

Package endpoint - 8.6.0 containing this change is available at https://epr.elastic.co

@pzl pzl deleted the Check_Ret_Addr_Outside_Image_Package branch November 22, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants