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

Initial documentation for environment variables #1680

Merged
merged 2 commits into from
Mar 15, 2021

Conversation

aabmass
Copy link
Member

@aabmass aabmass commented Mar 8, 2021

Description

Depends on #1671, must be rebased before merge

Live demo https://aabmass.github.io/opentelemetry-python/docs-envvar-demo/exporter/otlp/otlp.html

  • Centralized all the sphinx .. envvar:: docs into the API/SDK environment variable modules
  • Added .. envvar:: docstrings where they were missing, so they now show in the docs
  • Added references to that documentation elsewhere where the envvars are relevant (exporters, BSP, etc.)
  • Actually writing the docstrings is todo – i was thinking we could commit this and others can fill in what they know about over time

Part of #1147

Type of change

Please delete options that are not relevant.

  • This change requires a documentation update

How Has This Been Tested?

Built docs to see how it all looks. Here are some screenshots:

  • Screen Shot 2021-03-08 at 1 04 13 PM
  • Screen Shot 2021-03-08 at 1 05 47 PM
  • Sphinx index builds a full list of environment variables
    Screen Shot 2021-03-08 at 1 06 58 PM

Does This PR Require a Contrib Repo Change?

Answer the following question based on these examples of changes that would require a Contrib Repo Change:

  • The OTel specification has changed which prompted this PR to update the method interfaces of opentelemetry-api/ or opentelemetry-sdk/

  • The method interfaces of opentelemetry-instrumentation/ have changed

  • The method interfaces of test/util have changed

  • Scripts in scripts/ that were copied over to the Contrib repo have changed

  • Configuration files that were copied over to the Contrib repo have changed (when consistency between repositories is applicable) such as in

    • pyproject.toml
    • isort.cfg
    • .flake8
  • When a new .github/CODEOWNER is added

  • Major changes to project information, such as in:

    • README.md
    • CONTRIBUTING.md
  • Yes. - Link to PR:

  • No. (it can be done separate)

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Documentation has been updated

@aabmass aabmass requested review from a team, owais and toumorokoshi and removed request for a team March 8, 2021 18:01
@aabmass aabmass changed the title Envvar docs 1147 Document environment variables Mar 8, 2021
@aabmass aabmass marked this pull request as draft March 8, 2021 18:03
@owais
Copy link
Contributor

owais commented Mar 8, 2021

I'm not entirely sure about this. I think a global list of all support env vars is great but I don't think it's strange that a component doesn't document in detail every config env var it supports. For example, jaeger exporter now just lists the env vars it supports without explaining what they do or how to use them (types, values, etc). I realize this is easier for us but I highly doubt it'd be better for end users to jump between pages when all they want is to refer to docs for one component. Is there any easy way we an describe these in the component docs in addition to the global list?

@aabmass
Copy link
Member Author

aabmass commented Mar 8, 2021

I realize this is easier for us but I highly doubt it'd be better for end users to jump between pages when all they want is to refer to docs for one component.

I was going off of this comment mostly #1147 (comment). The links go directly to the env var documentation, which seems ok to me. Some of the envars don't apply to a specific piece, e.g. OTEL_EXPORTER_OTLP_TIMEOUT is relevant to span and metric exporters.

Is there any easy way we an describe these in the component docs in addition to the global list?

We could put the env var documentation along with the docs to get this, they still appear in the index the same. I'm not sure if there is a way to sort of expand out the link to copy the documentation inline, I'll look into it

@aabmass aabmass force-pushed the envvar-docs-1147 branch 3 times, most recently from f2a4dbf to ab4dd9c Compare March 8, 2021 22:19
@owais
Copy link
Contributor

owais commented Mar 8, 2021

Them linking to the other detail page is definitely nicer but I still feel having the docs inline on a page would be so much better if easily doable, otherwise this is fine.

@aabmass aabmass force-pushed the envvar-docs-1147 branch 3 times, most recently from 1733705 to 395abd2 Compare March 11, 2021 16:31
@aabmass aabmass changed the title Document environment variables Initial documentation for environment variables Mar 11, 2021
@aabmass aabmass marked this pull request as ready for review March 12, 2021 03:05
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks this is a nice improvement and sets up a good pattern for documenting env variables.

@lzchen lzchen merged commit 7662d83 into open-telemetry:main Mar 15, 2021
@aabmass aabmass deleted the envvar-docs-1147 branch March 16, 2021 21:08
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.

5 participants