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

DatadogLogs - Add Device's Brand, Name, and Model in LogEvent #1672

Merged
merged 5 commits into from
Mar 6, 2024

Conversation

aldoKelvianto
Copy link

What and why?

In the current logging implementation, we haven't automatically include device's brand, name, and model.
It is possible to add it manually by adding device-model in Global Attributes, as mentioned in this doc: https://docs.datadoghq.com/logs/log_collection/ios/?tab=cocoapods#global-attributes
However, as user in #1489 mentioned, it would be nice if the SDK add the device's model automatically.

Resolve #1489

How?

A brief description of implementation details of this PR.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests for Core, RUM, Trace, Logs, CR and WVT
  • Run unit tests for Session Replay
  • Run integration tests
  • Run smoke tests
  • Run tests for tools/

@maciejburda
Copy link
Contributor

Thanks for the contribution @aldoKelvianto

Could you please run linter, because it's failing on CI?

You can find more info under Lint section in this doc:
https://github.com/DataDog/dd-sdk-ios/blob/develop/CONTRIBUTING.md#lint

In the meantime I'll try to evaluate validity of your proposal and let you know. We have some internal action plan regarding this, and we need to make sure it's aligned.

@aldoKelvianto
Copy link
Author

Could you please run linter, because it's failing on CI?

Thank you so much for sharing the solution, I'll run the linter and try the CI again 🙏

In the meantime I'll try to evaluate validity of your proposal and let you know. We have some internal action plan regarding this, and we need to make sure it's aligned.

Sure thing, thank you!

maciejburda
maciejburda previously approved these changes Mar 6, 2024
@maciejburda
Copy link
Contributor

Enabled CI

@ncreated
Copy link
Collaborator

ncreated commented Mar 6, 2024

@aldoKelvianto great contribution, thank you! I made necessary updates for lint and tests and we will follow with a merge.

@DataDog/rum-mobile-ios All tests are green. CI failed on dependency-manager tests as MacCatalyst build doesn't support fork branches. This is false-positive, I will proceed with elevated permission merge.

maciejburda
maciejburda previously approved these changes Mar 6, 2024
@ncreated ncreated merged commit 0f04efe into DataDog:develop Mar 6, 2024
1 of 2 checks passed
@ncreated ncreated mentioned this pull request Mar 12, 2024
8 tasks
@maciejburda maciejburda mentioned this pull request Mar 19, 2024
8 tasks
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.

iOS Device Model, No Type
4 participants