open-telemetry / community

OpenTelemetry community content
https://opentelemetry.io
Apache License 2.0
756 stars 229 forks source link

REQUEST: Install Foresight on the GitHub organization #1263

Closed jpkrohling closed 1 year ago

jpkrohling commented 1 year ago

Affected Repository

https://github.com/open-telemetry/opentelemetry-collector and https://github.com/open-telemetry/opentelemetry-collector-contrib

Requested changes

We've been considering evaluating Foresight to get insights about our CI pipelines. We'd need this application installed as part of the GitHub organization. I would like therefore ask what the requirements are to have that done.

Reference: https://github.com/open-telemetry/opentelemetry-collector/issues/6245

Purpose

SaaS for CI insights.

Expected Duration

Potentially permanently. We'll request the uninstall in case we are unhappy with the tool.

Repository Maintainers

serkan-ozal commented 1 year ago

Hi folks,

Here is a Foresight public view example by pipenv (world famous packaging and virtual env management tool for python projects) as they have integrated Foresight into there CI pipelines and tests: https://pypa.app.runforesight.com/

jpkrohling commented 1 year ago

@tigrannajaryan, @bogdandrutu, who would be the right person to enable this for the collector repositories alone? Apparently, we do not need this enabled for the whole organization.

jpkrohling commented 1 year ago

Ping @bogdandrutu @tigrannajaryan

tigrannajaryan commented 1 year ago

Can you please add the link to the app to install? Is it this one https://github.com/marketplace/thundra-foresight?

Also, can you please tell what Github permissions does it require? I can't figure it out from the app's page.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

This is Serkan, CTO and Co-Founder of Thundra, which is the company behind Foresight.

We are happy to help you during onboarding and answer your questions.

The best way is to signup and create an account here: https://app.runforesight.com

Then, our onboarding page will redirect you to Github page to install our app. Then after install, you will be redirected to Foresight to select repositories to watch.

The required permissions are

serkan-ozal commented 1 year ago

Hi @tigrannajaryan @bogdandrutu @jpkrohling, Is there anything we (Foresight team) can provide, help or answer on this?

tigrannajaryan commented 1 year ago

@serkan-ozal can you please clarify why is write access needed by the app?

serkan-ozal commented 1 year ago

Hi @tigrannajaryan, It is for adding collected metrics to either check results or PR comments. But that behaviour can also be disabled.

tigrannajaryan commented 1 year ago

Hi @tigrannajaryan, It is for adding collected metrics to either check results or PR comments. But that behaviour can also be disabled.

@serkan-ozal do you mean it is possible to install the app in a way that does not require the write permissions or that write permissions will still be required, but adding collected metrics will be disabled?

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

Our app install will still require write permissions for PRs, issues and check-results but it will not be used by Foresight.

As I said, write permission is only required to add comment to PRs like showing executed test summary, etc ... but if this is a no go for you, we can look for alternative ways like custom app and so on ...

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

I have forgotten to say that these permissions are only for the repositories you selected while installing app. Even though Foresight Github app gets those permissions, it is not allowed to use those permissions at Github organization level for any repository unless you select repository individually during install.

tigrannajaryan commented 1 year ago

Thank you @serkan-ozal I will add this to the Technical Committees agenda next week to discuss and come back. My primary concern is the write permissions. It would be an easier task if the app only required read permissions.

serkan-ozal commented 1 year ago

Thanks @tigrannajaryan,

Waiting for your response. Sorry for bothering you

tigrannajaryan commented 1 year ago

Thanks @tigrannajaryan,

Waiting for your response. Sorry for bothering you

@serkan-ozal no problem, thanks for your comments.

tigrannajaryan commented 1 year ago

HI @serkan-ozal The TC is going to discuss this today. I wanted to see what the test result trends/history looks like in Foresight but cannot find them at https://pypa.app.runforesight.com/ I am looking for a UI that shows slow tests, tests that are flaky and keep failing, test error rate over time, etc. Can you point me to the right page that shows an example of this for some existing project?

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

I will go through Keycloak public page here: https://keycloak.app.runforesight.com/

You can go to test runs directly or over pull requests page: https://pypa.app.runforesight.com/repositories/github/pypa/pipenv/test-runs (for example you can filter test runs by status like Failed)

For example this is an example test run executed on main branch in Keycloak repository which has 28 failed tests. And here, you can see most erroneous and slowest test suites: https://keycloak.app.runforesight.com/repositories/github/keycloak/keycloak/testrun/67f9b063-15c5-3805-831b-fa4b4456970d

Then, when you click a test suite (for example ClientAuthSignedJWTTest), you will go to executed test cases in that test suite. And again you can see the most erroneous and slowest test cases here: https://keycloak.app.runforesight.com/repositories/github/keycloak/keycloak/testrun/67f9b063-15c5-3805-831b-fa4b4456970d/testsuite/org.keycloak.testsuite.oauth.ClientAuthSignedJWTTest

Then when you click a failed test (for example testCodeToTokenRequestSuccessPS256usingJwks) you will be redirected to its result page. Here, you can see the error logs of the failed test under Errors tab: https://keycloak.app.runforesight.com/repositories/github/keycloak/keycloak/testrun/67f9b063-15c5-3805-831b-fa4b4456970d/test/17697b8d-b522-4a87-b4d8-b26554ab4fda/errors

And when you click Performance tab, you will see historic results of that individual tests from the previous runs: https://keycloak.app.runforesight.com/repositories/github/keycloak/keycloak/testrun/67f9b063-15c5-3805-831b-fa4b4456970d/test/17697b8d-b522-4a87-b4d8-b26554ab4fda

tigrannajaryan commented 1 year ago

Thank you @serkan-ozal, this is very useful.

The TC discussed this and decided to try Foresight for one week on the Collector contrib repo. We would like @open-telemetry/collector-contrib-approvers to give feedback after one week of usage about how useful they find it.

The TC will also want to see what the app does with the write permissions during this trial period.

We will make the final call to keep it indefinitely or no after the trial week.

I will go ahead and start the signup process myself.

tigrannajaryan commented 1 year ago

I signed up and gave Foresigh permissions on https://github.com/open-telemetry/opentelemetry-collector-contrib

@open-telemetry/collector-contrib-approvers please take it from here.

I assume there is nothing else to be done by TC, so closing this issue. Please re-open if necessary.

tigrannajaryan commented 1 year ago

Collector contrib should be accessible via https://app.runforesight.com/repositories/github/open-telemetry/opentelemetry-collector-contrib/pull-requests

Aneurysm9 commented 1 year ago

I get redirected to https://app.runforesight.com/error when attempting to access the link above after granting authorization to the application.

serkan-ozal commented 1 year ago

Hi @Aneurysm9 @tigrannajaryan,

app.runforesight.com requires authorization but otel.app.runforesight.com is not as it is public view for the community as we assigned custom sub-domain and enabled public view for the community.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan, We (Foresight team) will also send another PR to the repository for adding 2 of our actions (workflow-kit and test-kit) to collect more data (resource usages, process traces, test reports, coverage reports, etc ...) to give more insights

Aneurysm9 commented 1 year ago

app.runforesight.com requires authorization but otel.app.runforesight.com is not as it is public view for the community as we assigned custom sub-domain and enabled public view for the community.

I delegated auth through GitHub's OAuth IdP and I am a member of @open-telemetry/collector-contrib-approvers. Shouldn't I have been able to view data at the location Tigran linked?

tigrannajaryan commented 1 year ago

Reopening to let the discussion continue.

serkan-ozal commented 1 year ago

Hi @Aneurysm9,

Currently Foresight organizations and Github organizations are not associated and linked on our side yet. So to access the OTEL Foresight account through app.runforesight.com as authenticated user, you need @tigrannajaryan to invite you to the OTEL Foresight organization by your email.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

Here is the doc link for you which shows how to invite a team member to your Foresight organization: https://docs.runforesight.com/features/team-management

serkan-ozal commented 1 year ago

And for the ones who don't need to access OTEL Foresight organization as authenticated user (for ex, the rest of the community), OTEL Foresight public view otel.app.runforesight.com can still be used

tigrannajaryan commented 1 year ago

Currently Foresight organizations and Github organizations are not associated and linked on our side yet. So to access the OTEL Foresight account through app.runforesight.com as authenticated user, you need @tigrannajaryan to invite you to the OTEL Foresight organization by your email.

That's a significant maintenance burden for Otel Admins (TC). We now have to maintain in parallel 2 permission structures, one in Github, another one in Foresight. This can be a dealbreaker.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

You're right, I totally understand the pain point. I will talk with the product team to prioritize organization sync between Foresight and Github. I will let you know here when are planning to deliver it

tigrannajaryan commented 1 year ago

Thanks @serkan-ozal

For now anyone who wants an invite DM me your email address.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

You can send an invite to me: serkan@thundra.io

In the meantime, we started discussing the Github and Foresight org sync feature with the product team. At this point, I want to ask you that we are thinking of getting readonly permission to fetch organization members. Does it sound a reasonable permission and way for you?

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

What do you think about readonly permission to fetch organization members. Does it sound reasonable approach to you?

Also we have opened this PR https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/16427 to add Foresight Github actions into your workflows to provide richer workflow and test insights.

tigrannajaryan commented 1 year ago

What do you think about readonly permission to fetch organization members. Does it sound reasonable approach to you?

Readonly sounds good, but adding all organization members as users of the project is probably not what we want, depending on what rights the users in Foresight have. Normally we want only Maintainers or Approvers of a particular github repo to have additional permissions in Foresight.

Otel has hundreds of regular organization members who should not have any special permissions on Foresight.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

We are thinking of adding all members in your OTEL Github organization to Foresight OTEL organization as "USER" (by default) who will have readonly access to your Foresight account. Then you can upgrade some member's roles to "ADMIN" in Foresight (from Foresight UI) individually so they will have special permissions.

Does it make sense?

tigrannajaryan commented 1 year ago

We are thinking of adding all members in your OTEL Github organization to Foresight OTEL organization as "USER" (by default) who will have readonly access to your Foresight account.

Can you please clarify what additional rights does the "USER" have compared to someone who is not a user? Since this is a public project it looks like anyone can see the Foresight results, even if they are not added as a "USER", right?

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

While anonymous users cannot see private repositories in Foresight, "USER"s can also see insights from private repositories. But since you only watch a public repository in Foresight, for your case in practice there is no difference between authenticated user (has "USER" role) and anonymous user (using public view, so no authentication).

serkan-ozal commented 1 year ago

@tigrannajaryan,

For your case, is it OK to auto invite every member in your Github org to your Foresight org as "USER" (so you can promote them individually) or do you want to invite only some users which have specific role?

By the way, as far as I know, Github has only "Owner" and "Member" roles at organization level. So since we will fetch members at organization level, we won't see repo level roles like "Maintainers" or "Approvers".

tigrannajaryan commented 1 year ago

@codeboten @jpkrohling you asked me to invite you to Foresight. Based on what @serkan-ozal wrote above you should have been able to access it without being invited. Wasn't this the case?

jpkrohling commented 1 year ago

Great question. I just tried opening https://otel.app.runforesight.com/highlights in a clean browser, and I was able to browse it while not even logged in, which is exactly how it should be.

tigrannajaryan commented 1 year ago

Great question. I just tried opening https://otel.app.runforesight.com/highlights in a clean browser, and I was able to browse it while not even logged in, which is exactly how it should be.

If this is the case then we do not need to add Otel members as Foresight users, there is no benefit in doing that.

What I would like to understand is who needs any additional permissions in Foresight. @serkan-ozal do I understand it correctly that the only granularity of permission we have in Foresight is "user" vs "admin" for the entire org. What is "admin" capable of doing that is relevant for Otel? Looks like Billing doesn't apply.

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

Here are the differences between "USER" and "ADMIN" in Foresight:

Admins

Users

serkan-ozal commented 1 year ago

Hi @tigrannajaryan,

Currently, you can invite some members individually with "ADMIN" role into your Foresight org and others can access OTEL insights as anonymous user (no login/signup is required) through public view here: otel.app.runforesight.com

So is it still required for you to auto invite your Github members into Foresight? As far as I understood from what you wrote above, current user management capabilities meet your current needs. Is it correct?

serkan-ozal commented 1 year ago

Hi @tigrannajaryan @jpkrohling,

Sorry for pinging you over the weekend :)

What do you think about this? Is current behavior OK for you? or are you looking an improvement from us on this?

Currently, you can invite some members individually with "ADMIN" role into your Foresight org and others can access OTEL insights as anonymous user (no login/signup is required) through public view here: otel.app.runforesight.com

So is it still required for you to auto invite your Github members into Foresight? As far as I understood from what you wrote above, current user management capabilities meet your current needs. Is it correct?

jpkrohling commented 1 year ago

I believe the current behavior is fine, and we already have enough to get started.

codeboten commented 1 year ago

@serkan-ozal is it possible to get links from github directly to foresight? this would make finding the logs easier.

For example how would one go from the failure here: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/3570199022/jobs/6001231808

To https://otel.app.runforesight.com/repositories/github/open-telemetry/opentelemetry-collector-contrib/testrun/824f291b-1fa2-3df1-917b-16e64341f4e4/test/6deb59ed-2045-48ae-8eeb-a9e234e30522/errors?

serkan-ozal commented 1 year ago

Hi @codeboten,

We have released PR comment feature today. In the PR comment, we provide many insights for the workflows and tests with the links to the Foresight. I think, it is the thing you are looking for. But it is disabled by default.

Do you want us to enable PR comment for OTEL account @codeboten @tigrannajaryan @jpkrohling?

jpkrohling commented 1 year ago

Yes, let's give it a try!

ghost commented 1 year ago

I believe the current behavior is fine, and we already have enough to get started

Sent from Yahoo Mail for iPhone

On Monday, November 28, 2022, 5:07 PM, Juraci Paixão Kröhling @.***> wrote:

I believe the current behavior is fine, and we already have enough to get started.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

jpkrohling commented 1 year ago

@serkan-ozal, I like the new comment, but a small bug prevents links from working. Look at this issue:

https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/16357

Under "See details on Foresight", it has the following link:

https://otel.app.runforesight.com/repositories/github/open-telemetry/opentelemetry-collector-contrib/pull-request/16,357

Note that the final number has a comma, causing the data to never complete loading. Removing the comma makes it work.

serkan-ozal commented 1 year ago

Hi @jpkrohling,

We have got its alert :) and working on the fix. It is indeed a problem caused while generating link.