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

[ddtelemetry] Prefix ffi functions with ddog_telemetry #367

Merged

Conversation

paullegranddc
Copy link
Contributor

What does this PR do?

  • Rename ddtelemetry ffi functions from ddog_handle_* to ddog_telemetr_handle_* and ddog_builder_* to ddog_telemetry_builder_*

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

How to test the change?

Describe here in detail how the change can be validated.

For Reviewers

  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

…_handle and ddog_builder to ddog_telemetry_builder
@ivoanjo
Copy link
Member

ivoanjo commented Mar 21, 2024

Not sure if you want to take the opportunity here to tweak, but for profiling stuff, we usually use a slightly different naming where we use struct names with camel case, e.g. (ddog_Vec_Tag or ddog_prof_Endpoint.

@paullegranddc
Copy link
Contributor Author

paullegranddc commented Mar 21, 2024

@ivoanjo
Right now, this PR is more about removing confusion and potential collisions on the telemetry ffi, since I just need the right part of my brain convince the left one to make that happen.

If we want to standardise function and type names for all of libdatadog, we probably should write a small rfc now that we have a directory for that, and share it with the profiling and data-pipelines team?

@paullegranddc paullegranddc merged commit 342a0fb into main Mar 21, 2024
19 of 20 checks passed
@paullegranddc paullegranddc deleted the paullgdc/ddtelemetry/prefix_telemetry_ffi_function branch March 21, 2024 14:13
@ivoanjo
Copy link
Member

ivoanjo commented Mar 21, 2024

We actually did discuss such standardization shortly before 1.0, see #73 and https://docs.google.com/spreadsheets/d/1iuuSCJ0NcSOpo2RVvkQ8f8ugz2XlEbbpFZLVOg6WioE/edit#gid=0 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants