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 device.manufacturer to semantic conventions for resources #2100

Merged
merged 8 commits into from
Dec 16, 2021

Conversation

ladd
Copy link
Contributor

@ladd ladd commented Nov 4, 2021

Changes

  • Proposed manufacturer field

@ladd ladd requested review from a team as code owners November 4, 2021 21:00
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 4, 2021

CLA Signed

The committers are authorized under a signed CLA.

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 16, 2021
@ladd
Copy link
Contributor Author

ladd commented Nov 16, 2021

Could I get a review, please?

@arminru arminru added area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory labels Nov 16, 2021
@arminru arminru changed the title Update device.md Add device.manufacturer to semantic conventions for resources Nov 16, 2021
@arminru
Copy link
Member

arminru commented Nov 16, 2021

@ladd Please note that you will need to update the YAML definitions in this folder following the readme there:
https://github.com/open-telemetry/opentelemetry-specification/tree/main/semantic_conventions
By running the command documented there, the markdown table you edited manually will be generated automatically an make the build pass.

@ladd
Copy link
Contributor Author

ladd commented Nov 16, 2021

@ladd Please note that you will need to update the YAML definitions in this folder following the readme there: https://github.com/open-telemetry/opentelemetry-specification/tree/main/semantic_conventions By running the command documented there, the markdown table you edited manually will be generated automatically an make the build pass.

Thanks -- done!

@Oberon00
Copy link
Member

Is this something we'd actually want to send? Since we already send device IDs, couldn't this be handled with a device database at the backend? Or do you think the practical advantages will be enough? Does one usually have an API that the instrumentation could call to get the manufacturer? Or would it have to have a DB?

@ladd
Copy link
Contributor Author

ladd commented Nov 16, 2021

Device ids are typically not mappable to manufacturer strings, unless you have a separate instrumentation mechanism to populate a DB. Device model strings can sometimes be mapped, but it normally requires licensing a proprietary database (like GSM Arena) I do think the practical advantages are there -- specific Android manufacturers often tweak the OS in compatibility breaking ways. The API exists for Android -- see my footnote in the PR. For iOS, you would normally just hardcode Apple.

@github-actions github-actions bot removed the Stale label Nov 17, 2021
@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 25, 2021
@ladd
Copy link
Contributor Author

ladd commented Nov 26, 2021

Can I get a review / merge if the comments have been addressed?

Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

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

Please add a CHANGELOG entry.

semantic_conventions/resource/device.yaml Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the Stale label Nov 27, 2021
@ladd
Copy link
Contributor Author

ladd commented Dec 6, 2021

Can I get a review / merge if the comments have been addressed?

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 14, 2021
@jsuereth jsuereth enabled auto-merge (squash) December 15, 2021 17:08
auto-merge was automatically disabled December 15, 2021 17:57

Head branch was pushed to by a user without write access

@jsuereth jsuereth enabled auto-merge (squash) December 16, 2021 00:21
@jsuereth jsuereth merged commit 86b4585 into open-telemetry:main Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants