-
Notifications
You must be signed in to change notification settings - Fork 37
OCI APM Support with Traceloop library #68
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
Conversation
OCI APM Support with Traceloop library
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.
❌ Changes requested. Reviewed everything up to c44e50d in 1 minute and 41 seconds
More details
- Looked at
67
lines of code in1
files - Skipped
1
files when reviewing. - Skipped posting
6
drafted comments based on config settings.
1. openllmetry/integrations/oraclecloud.mdx:16
- Draft comment:
Use straight ASCII quotes rather than fancy quotes for APM_BASE_URL. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment appears to be incorrect - it's suggesting a change that's already implemented. The code uses straight ASCII quotes, not fancy quotes. This seems like either a mistake by the automated tool or it's seeing something I'm not seeing. Since the code already follows the suggested practice, this comment is not useful.
Could there be some invisible Unicode characters that I'm not seeing? Could the quotes look straight in my viewer but actually be fancy quotes in the file?
Even if there were invisible characters, the diff shows the line as new code being added, and it clearly shows straight quotes in the diff. The automated tool should be working from the same diff view.
The comment should be deleted as it suggests making a change that is already implemented in the code.
2. openllmetry/integrations/oraclecloud.mdx:18
- Draft comment:
Replace fancy quotes with standard quotes for APM_SERVICE_NAME. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The comment appears to be incorrect - the code already uses standard straight quotes. Either the comment is addressing an issue that was already fixed, or it's seeing something that isn't there. Looking at the full file context, all other string quotes are also standard quotes.
Could there be some invisible Unicode characters that aren't displaying properly in my view? Could the diff be showing the post-fix state rather than the pre-fix state?
Even if there were invisible characters, the suggested fix shows exactly the same text as what's already there, so the comment appears redundant or incorrect either way.
The comment should be deleted as it appears to be either incorrect or addressing an issue that's already resolved.
3. openllmetry/integrations/oraclecloud.mdx:16
- Draft comment:
Replace curly quotes with standard quotes to ensure valid copy-paste code. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment appears to be incorrect - there are no curly quotes to replace. The code already uses standard straight quotes which is correct. Making this change would not actually change anything. The comment may be seeing a font rendering issue or making an incorrect assumption.
Could there be curly quotes that aren't visible in this rendering of the code? Could this be a valid concern about potential copy-paste issues?
Even if there were invisible curly quotes, the suggestion shows the exact same string that's already there. And the code appears to be properly formatted markdown code blocks which should handle quotes correctly.
The comment appears to be incorrect as there are no curly quotes to fix, and the suggested change is identical to the existing code.
4. openllmetry/integrations/oraclecloud.mdx:17
- Draft comment:
Ensure consistent casing in data key strings ('datakey' vs 'dataKey'). - Reason this comment was not posted:
Marked as duplicate.
5. openllmetry/integrations/oraclecloud.mdx:16
- Draft comment:
It looks like the dunder quotes in these lines (e.g.“My LLM Service”
and“<OCI APM dataUploadEndpoint>/20200101/opentelemetry/private"
) are typographic (curly) quotes. Please replace them with standard ASCII double quotes ("
) to ensure consistency and prevent potential issues. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The comment appears to be incorrect - I don't see any curly quotes in the code examples. All quotes appear to be standard ASCII quotes. Even if there were curly quotes, this is a documentation file, not actual code, so it would be less critical. The comment seems to be raising a non-existent issue.
Could I be missing some subtle Unicode characters that look like standard quotes but aren't? Should documentation code examples be held to the same standard as actual code?
Even if there were subtle Unicode differences, the code examples are clearly readable and since this is documentation, not actual code that gets executed, this level of scrutiny about quote characters is unnecessary.
The comment should be deleted because it appears to be incorrect - there are no curly quotes to fix - and even if there were, it would be an overly pedantic concern for a documentation file.
6. openllmetry/integrations/oraclecloud.mdx:2
- Draft comment:
In the title on Line 2 and similarly in the link text on Line 11, consider adding a space before the opening parenthesis in phrases such as "Application Performance Monitoring(APM) service". For improved readability, it should be "Application Performance Monitoring (APM) service". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
This is a purely stylistic change about spacing in documentation. While technically correct from a typography perspective, it's a very minor issue. The rules specifically state not to make purely informative comments or comments about obvious/unimportant things. This seems to fall into that category.
The spacing issue could affect readability of the documentation, which is important for users. Also, it appears in multiple places, suggesting it might be a systematic issue.
While documentation readability matters, this is an extremely minor formatting issue that doesn't impact understanding. The rules clearly state not to make obvious or unimportant comments.
This comment should be deleted as it's too minor and falls under the category of unimportant formatting suggestions.
Workflow ID: wflow_FwaKMy0RGWHJ5KEh
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
|
||
```python | ||
APM_BASE_URL=“<OCI APM dataUploadEndpoint>/20200101/opentelemetry/private" | ||
APM_DATA_KEY="datakey <OCI APM Private Data Key>" |
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.
Ensure consistent casing for data key (e.g., datakey
vs dataKey
) across examples.
APM_DATA_KEY="datakey <OCI APM Private Data Key>" | |
APM_DATA_KEY="dataKey <OCI APM Private Data Key>" |
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.
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.
@Anand-GitH, I have addressed your comments in pull request #69
You can configure Ellipsis to address comments with a direct commit or a side PR, see docs.
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.
Thanks @Anand-GitH!
OCI APM Support with Traceloop library
Important
Adds documentation for integrating OCI APM with Traceloop library using OpenTelemetry in
oraclecloud.mdx
.oraclecloud.mdx
for integrating OCI APM with Traceloop library.This description was created by
for c44e50d. It will automatically update as commits are pushed.