Skip to content

Conversation

@ptiurin
Copy link
Contributor

@ptiurin ptiurin commented May 31, 2022

Tracking usage based on parsing the stack and detecting known integration signatures of file + function name.
Also allowing access for manual override/addition of connector information.

Addition of a UsageTracker instance to the Conection causes a diffferent garbage collection to trigger, therefore a warnings test also had to be reworked with a manual call to gc. Users still get a warning of an unclosed connection if their script exits without doing so. This change only affects how we test the warnings, not how the users of the SDK see them.

@ptiurin ptiurin requested a review from stepansergeevitch June 6, 2022 19:40
Copy link
Collaborator

@stepansergeevitch stepansergeevitch left a comment

Choose a reason for hiding this comment

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

LGTM, left a couple of comments

connectors = detect_connectors()
logger.debug("Detected running from packages: %s", str(connectors))
# Override auto-detected connectors with info provided manually
if type(custom_connectors) == tuple:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if someone would pass

(
    ("connector1", "version1"),
    ("connector2", "version2")
)

Copy link
Collaborator

@stepansergeevitch stepansergeevitch Jun 7, 2022

Choose a reason for hiding this comment

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

Maybe let's just enforce passing a list? And don't allow passing a single item

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

96.2% 96.2% Coverage
0.0% 0.0% Duplication

@ptiurin ptiurin merged commit 5528d1b into main Jun 8, 2022
@ptiurin ptiurin deleted the usage-tracking branch June 8, 2022 09:53
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.

3 participants