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

docs(messaging): add gcp_pubsub as a messaging system #490

Merged
merged 7 commits into from
Nov 13, 2023

Conversation

alevenberg
Copy link
Contributor

@alevenberg alevenberg commented Nov 3, 2023

Changes

Adds gcp_pubsub as a messaging system value

Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.

Merge requirement checklist

Waiting to sign the CLA until the PR is created.
Also I ran make fix, but it generated some diffs in CONTRIBUTING.md (* -> -) and CHANGELOG.md (whitespace), so I left those out.

Skipped, since it is a trivial change

Skipped since it only affects the docs

Copy link

linux-foundation-easycla bot commented Nov 3, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@alevenberg
Copy link
Contributor Author

Disclaimer: I'm not familiar with Otel Semantic Conventions.

Is there a place to specify a semantic convention for a value (not a key). This value (gcp_pubsub) will be used in multiple languages. Is there a place we can define it upstream so it appears in a file like opentelemetry/trace/semantic_conventions.h(for C++)?

@pyohannes
Copy link
Contributor

Hi @alevenberg . I'd recommend you introduce a dedicated document for GCP PubSub, as it exists for Kafka or RabbitMQ. There you define additional requirements for GCP PubSub instrumentation, like it's done here: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/messaging/rocketmq.md?plain=1#L13

You might just start out with defining the value for messaging.system, you can then add definitions for more system-specific attributes there if needed. Ideally you (or somebody else with domain knowledge) would then be willing to own and maintain this document specific to GCP PubSub.

Is there a place we can define it upstream so it appears in a file like opentelemetry/trace/semantic_conventions.h(for C++)?

No, that's still a manual process and requires changes for each supported language.

@alevenberg
Copy link
Contributor Author

Hi @alevenberg . I'd recommend you introduce a dedicated document for GCP PubSub, as it exists for Kafka or RabbitMQ. There you define additional requirements for GCP PubSub instrumentation, like it's done here: main/docs/messaging/rocketmq.md?plain=1#L13

You might just start out with defining the value for messaging.system, you can then add definitions for more system-specific attributes there if needed. Ideally you (or somebody else with domain knowledge) would then be willing to own and maintain this document specific to GCP PubSub.

Sounds good! I added it, and I will add more information about tracing semantic conventions in GCP Pub/Sub. I can own this document for now.

Is there a place we can define it upstream so it appears in a file like opentelemetry/trace/semantic_conventions.h(for C++)?

No, that's still a manual process and requires changes for each supported language.

Ack, thanks for answering!

@alevenberg alevenberg marked this pull request as ready for review November 7, 2023 22:00
@alevenberg alevenberg requested review from a team November 7, 2023 22:00
@arminru arminru requested a review from jsuereth November 8, 2023 09:00
@alevenberg
Copy link
Contributor Author

cc @dbolduc

Copy link

@dbolduc dbolduc left a comment

Choose a reason for hiding this comment

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

Should we link to gcp-pubsub.md in /docs/messaging/README.md ?

Technology specific semantic conventions are defined for the following messaging systems:
* [Kafka](kafka.md): Semantic Conventions for *Apache Kafka*.
* [RabbitMQ](rabbitmq.md): Semantic Conventions for *RabbitMQ*.
* [RocketMQ](rocketmq.md): Semantic Conventions for *Apache RocketMQ*.

@alevenberg
Copy link
Contributor Author

alevenberg commented Nov 9, 2023

Should we link to gcp-pubsub.md in /docs/messaging/README.md ?

Technology specific semantic conventions are defined for the following messaging systems:
* [Kafka](kafka.md): Semantic Conventions for *Apache Kafka*.
* [RabbitMQ](rabbitmq.md): Semantic Conventions for *RabbitMQ*.
* [RocketMQ](rocketmq.md): Semantic Conventions for *Apache RocketMQ*.

Good catch. Done

@alevenberg
Copy link
Contributor Author

alevenberg commented Nov 9, 2023

I ran make table-generation to generate the markdown files from the yaml

Copy link
Member

@joaopgrassi joaopgrassi left a comment

Choose a reason for hiding this comment

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

Please also add a changelog entry

model/registry/messaging.yaml Show resolved Hide resolved
docs/messaging/gcp-pubsub.md Outdated Show resolved Hide resolved
Copy link
Member

@joaopgrassi joaopgrassi left a comment

Choose a reason for hiding this comment

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

Just the changelog entry missing now.

@arminru arminru merged commit 30ea09f into open-telemetry:main Nov 13, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants