-
Notifications
You must be signed in to change notification settings - Fork 3
Instrument the transformer using the opentelemetry-sdk #727
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #727 +/- ##
==========================================
+ Coverage 92.85% 93.23% +0.38%
==========================================
Files 9 9
Lines 532 562 +30
Branches 110 112 +2
==========================================
+ Hits 494 524 +30
Misses 21 21
Partials 17 17
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm no expert at naming things, but do the suggestions make sense? Otherwise LGTM!
Here is a link to the convention: otel-metrics-semantic-conventions
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
6e32f6c
to
6e1f652
Compare
Signed-off-by: Janine Olear <[email protected]>
6e1f652
to
8403c09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great foundation, I can't wait to see the rest of the implementation! I only got some really minor questions that have no impact on functionality. Feel free to merge once you're ready.
@@ -518,7 +540,7 @@ class TransformerV2ListVersionByProvider(TransformerV2): | |||
"""Generate a list for all available versions for a specific provider.""" | |||
|
|||
def run(self, data: list[DataEntry]) -> list: | |||
# Start each version at a count of 0 so we can increment the counter as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this was removed by mistake?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol - yes you are right! :D
@@ -41,6 +41,8 @@ | |||
help="List of predefined files to process", | |||
) | |||
def run(origin_path: str, destination_path: str, arg_files: str, filter_until: str) -> None: | |||
# metrics.get_meter_provider().start_pipeline(ConsoleMetricExporter(), interval=5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a placeholder for future implementations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the next step is to export the data.
@@ -518,7 +540,7 @@ class TransformerV2ListVersionByProvider(TransformerV2): | |||
"""Generate a list for all available versions for a specific provider.""" | |||
|
|||
def run(self, data: list[DataEntry]) -> list: | |||
# Start each version at a count of 0 so we can increment the counter as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol - yes you are right! :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Late, but it looks good to me :-)
This PR instruments:
While I have still an open question how to call the counter add method when data gets filtered using lambda.
As a next step, we have to add a otel exporter to store and visualize the data.
closes #728