Skip to content

python configuration prototype#44

Closed
codeboten wants to merge 7 commits intomainfrom
codeboten/py-prototype
Closed

python configuration prototype#44
codeboten wants to merge 7 commits intomainfrom
codeboten/py-prototype

Conversation

@codeboten
Copy link
Copy Markdown
Collaborator

@codeboten codeboten commented Feb 10, 2023

Here's the start of the python prototype. Specifically here, I wanted to test out the ergonomics of using the dictionary object parsed from the config file. I've updated a couple of things in the schema/config file as a result which I'm not opposed to changing back but here's the list of changes:

  1. changed propagators to an array of strings instead of objects. propagators are currently identified by a string in the spec. This means in theory users would be limited in what they can configure in a propagator, but that's the state of the world today.
  2. renamed *_provider to the signal name instead. i think this makes understand the configuration a bit more ergonomic since for example we're configuring exporters for the signal, and then applying them to a specific processor. the user may not need to know this is configuring the provider specifically.
  3. changed processors to the same format as the exporter with type/identifier naming

The code as is supports configuring a console exporter, and turning on/off the SDK. I will continue adding more interpretation of the configuration in the future. The only interface users would interact with is shown in prototype.py as the following:

    otel.configure(otel.parse_and_validate_from_config_file(sys.argv[1], sys.argv[2]))

ocelotl
ocelotl reviewed Feb 13, 2023
View reviewed changes
prototypes/python/otel.py Outdated
@@ -0,0 +1,119 @@

import importlib
Copy link
Copy Markdown

@ocelotl ocelotl Feb 13, 2023

Choose a reason for hiding this comment

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

Nit, use from x import y

@martinkuba
Copy link
Copy Markdown
Contributor

martinkuba commented Feb 13, 2023

Question about the processor names - what is the reason that the name includes the exporter name after the slash? I can see it as a more descriptive name, but the type of the processor will have to be parsed from the name.

@codeboten
Copy link
Copy Markdown
Collaborator Author

codeboten commented Feb 13, 2023

Question about the processor names - what is the reason that the name includes the exporter name after the slash? I can see it as a more descriptive name, but the type of the processor will have to be parsed from the name.

It's probably not necessary, since the name is really identifying the type of processor this is and not used as a unique identifier anywhere else. I suppose it could make returning an error when configuring a specific processor a bit more easy to identify, but it's probably not needed at this time

@codeboten codeboten force-pushed the codeboten/py-prototype branch from c67d0fe to e45b943 Compare February 15, 2023 04:21
Alex Boten added 7 commits February 21, 2023 14:39
python config prototype
f6054e5 
Signed-off-by: Alex Boten <aboten@lightstep.com>
a few ergonomics changes
54f5255 
Updated the list of propagators to an array of strings. Renamed *_provider to the name of the signal instead. This makes searching the parsed configuration a bit cleaner, since exporters are configured per signal, and not necessarily per provider (at least conceptually i found it easier to grok).

Signed-off-by: Alex Boten <aboten@lightstep.com>
more work to get prototype working
d7e9916 
Signed-off-by: Alex Boten <aboten@lightstep.com>
change object to array
c3006c9
use entry points where possible
a0f95ed 
Signed-off-by: Alex Boten <aboten@lightstep.com>
update prototype to test out otlp, added insecure field
e123d14 
Signed-off-by: Alex Boten <aboten@lightstep.com>
add meter provider configuration
bacb1d4 
Signed-off-by: Alex Boten <aboten@lightstep.com>
@codeboten codeboten force-pushed the codeboten/py-prototype branch from 159b1b5 to bacb1d4 Compare February 21, 2023 23:38
ocelotl
ocelotl reviewed Feb 23, 2023
View reviewed changes
prototypes/python/otel.py

import logging
from typing import List, Sequence, Tuple
from pkg_resources import iter_entry_points
Copy link
Copy Markdown

@ocelotl ocelotl Feb 23, 2023

Choose a reason for hiding this comment

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

pkg_resources has been deprecated (see this, and also this).

Copy link
Copy Markdown
Collaborator Author

@codeboten codeboten Feb 23, 2023

Choose a reason for hiding this comment

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

Ah good to know. It's probably ok for the prototype but wouldn't want to ship this code :D

ocelotl
ocelotl reviewed Feb 23, 2023
View reviewed changes
prototypes/python/otel.py
def _resource(self):
# resource detection
# attributes
from opentelemetry.sdk.resources import Resource
Copy link
Copy Markdown

@ocelotl ocelotl Feb 23, 2023

Choose a reason for hiding this comment

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

Why import here?

Copy link
Copy Markdown
Collaborator Author

@codeboten codeboten Feb 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

not necessary to have the import in the method, that was just me being lazy

ocelotl
ocelotl reviewed Feb 23, 2023
View reviewed changes
Copy link
Copy Markdown

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

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

Just some comments

@codeboten codeboten mentioned this pull request Feb 28, 2023
Merged
@codeboten codeboten mentioned this pull request Sep 18, 2023
Open
@codeboten
Copy link
Copy Markdown
Collaborator Author

codeboten commented Jan 12, 2024

Closing in favour of open-telemetry/opentelemetry-configuration#54

@codeboten codeboten closed this Jan 12, 2024
@codeboten codeboten deleted the codeboten/py-prototype branch January 12, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ocelotl ocelotl ocelotl left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@codeboten @martinkuba @ocelotl