Skip to content
This repository has been archived by the owner on Nov 3, 2024. It is now read-only.

Add blackfire example #5

Merged
merged 5 commits into from
Aug 19, 2019
Merged

Add blackfire example #5

merged 5 commits into from
Aug 19, 2019

Conversation

susannemoog
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Aug 2, 2019

CLA assistant check
All committers have signed the CLA.

@rfay
Copy link
Member

rfay commented Aug 2, 2019

I imagine you're already working on this, but please add a README.md with the full setup instructions, and an entry in the top-level README.md. Thanks!

@susannemoog
Copy link
Contributor Author

I imagine you're already working on this, but please add a README.md with the full setup instructions, and an entry in the top-level README.md. Thanks!

Added instructions that are hopefully understandable.

@rfay
Copy link
Member

rfay commented Aug 2, 2019

You're awesome, I'll look forward to trying it out.

I'm a never-ever with Blackfire. Should we/could we add at least a link to what you would do to try it out, how you'd know it was working?

@susannemoog
Copy link
Contributor Author

I'm unsure what would be needed. Basically after you followed the instructions (and added the client as described on the blackfire web page link) you should be able to start profiling requests either with the firefox or chrome addon or with the cli client as described here: https://blackfire.io/docs/cookbooks/profiling-http - does that help or do you need more info? if that's enough I can update the readme. if you need anything more, feel free to hit me up on TYPO3 slack.

Copy link
Collaborator

@mglaman mglaman left a comment

Choose a reason for hiding this comment

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

This matches my setup and the one discussed in the main DDEV issues

@rfay
Copy link
Member

rfay commented Aug 12, 2019

@mglaman @susannemoog I worked through this as a Blackfire n00b and made a number of updates in 8dc1fb2 which I'd appreciate if you'd review.

I'm not entirely convinced that we need the blackfire agent in a separate container, but whatever. Seems to me like we could just webimage_extra_packages: [blackfire-agent] and provide the config via environment variables as we're already doing. But it's fine like this I guess.

@susannemoog
Copy link
Contributor Author

@rfay something weird seems to have happened in the basic usage section - I guess you wanted to delete that? Additionally, if you are mentioning the chrome extension, should we also mention the one for firefox? (https://blackfire.io/docs/integrations/firefox )

And no, it's not strictly necessary to run the agent in a separate container, however it's cleaner and I think clearer that way, as in theory you'd need only one running agent for many projects connecting to it (and especially when thinking production setups and the like, making it clear that the agent is a separate service is nicer, I think).

@rfay
Copy link
Member

rfay commented Aug 13, 2019

Something definitely happened, sorry! I'll fix.

@rfay
Copy link
Member

rfay commented Aug 13, 2019

Fixed up basic usage in readme

@susannemoog
Copy link
Contributor Author

Looks better now. What about the firefox addon? Or do you deem it not important enough? It's fine either way from my side.

@rfay
Copy link
Member

rfay commented Aug 16, 2019

Sorry, I just forgot. Feel free to push a commit in there. Thanks for the review and the pr!

@rfay rfay merged commit a9d31bc into ddev:master Aug 19, 2019
@rfay
Copy link
Member

rfay commented Aug 19, 2019

Thanks so much for this!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants