Skip to content

Implement Unit Test for AWS Metric Attributes Span Exporter #30

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

Merged
merged 42 commits into from
Jan 30, 2024

Conversation

XinRanZhAWS
Copy link
Contributor

Description of changes:
Implement Unit Test for AWS Metric Attributes Span Exporter

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@XinRanZhAWS XinRanZhAWS requested a review from a team January 25, 2024 19:07
Copy link
Contributor

@zzhlogin zzhlogin left a comment

Choose a reason for hiding this comment

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

Please go over the file:

  1. add type hint for all variables when it is first initialized.
  2. add all comments the same as Java tests.
  3. There should be no module level function, please move them into the class and mark as internal use starting with _. Same applies for variables.

Copy link
Contributor

@zzhlogin zzhlogin left a comment

Choose a reason for hiding this comment

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

Please add one more test to address this :

# TODO: AwsMetricAttributesSpanExporter depends on internal ReadableSpan method _attributes.

Copy link
Contributor

@zzhlogin zzhlogin left a comment

Choose a reason for hiding this comment

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

I only add comments for partial of the files. Please go over the file carefully, and add the type hint for all variable initialization that is missing .

@XinRanZhAWS XinRanZhAWS merged commit 928a5be into main Jan 30, 2024
@XinRanZhAWS XinRanZhAWS deleted the unittest_test_aws_metric_attributes_span_exporter branch January 30, 2024 23:37
XinRanZhAWS pushed a commit that referenced this pull request Feb 7, 2024
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.

2 participants